From 93c160ec513236f8645640d77a3ecdcd87ed5273 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Sun, 16 Aug 2026 20:30:07 -0700 Subject: [PATCH] Add agent restart lifecycle command Extends the existing CheckIn RPC with a one-shot AgentCommand (restart only -- stop/uninstall need real per-platform OS service-manager integration and stay deliberately out of scope), delivered at-most-once: cleared the instant it's handed to the agent in a response, since a restarting agent's process is gone before it could ever confirm receipt. On restart, the agent flushes whatever's buffered, aborts its source task, and exits cleanly, relying entirely on the host's own service manager to bring it back up. Issuing a command is gated at RoleAdmin (stricter than config editing's RoleEditor) and logged into the same audit_log table Phase 7's AI interactions use, via a new agent_command event type. A real bug was found and fixed during live verification: the first implementation tried to atomically read-and-clear pending_command in a single INSERT...ON CONFLICT statement using a sibling CTE referenced only from RETURNING, on the assumption that Postgres evaluates every part of a WITH query against one pre-statement snapshot. That's wrong specifically for FOR UPDATE, which always reads the latest row version including one written earlier in the same statement -- confirmed empirically (a restart command was always coming back empty even when genuinely pending, so the agent never received it). Fixed by splitting into two real, ordered statements inside one explicit transaction. See /docs/agent-management-design.md's "Lifecycle commands" section. --- agent/sentry-agent/src/main.rs | 22 ++- api/agents/handler.go | 73 +++++++++- api/agents/handler_test.go | 99 +++++++++++++- api/agents/store.go | 54 +++++++- api/cmd/api/main.go | 5 +- docs/agent-management-design.md | 101 ++++++++++++-- enterprise/cmd/enterprise-api/main.go | 6 +- .../internal/audit/agent_command_adapter.go | 59 ++++++++ enterprise/internal/audit/audit.go | 8 ++ .../internal/agentregistry/agentregistry.go | 60 +++++++-- ingest/internal/grpcserver/server.go | 60 ++++++--- ingest/internal/grpcserver/server_test.go | 39 +++++- .../0038_add_agent_pending_command.sql | 18 +++ .../0039_add_agent_command_event_type.sql | 9 ++ proto/sentry/agent/v1/agent_control.pb.go | 126 +++++++++++++++--- proto/sentry/agent/v1/agent_control.proto | 30 +++++ web/src/lib/api.ts | 20 +++ web/src/routes/agents/[host]/+page.svelte | 58 +++++++- 18 files changed, 775 insertions(+), 72 deletions(-) create mode 100644 enterprise/internal/audit/agent_command_adapter.go create mode 100644 metadata/migrations/0038_add_agent_pending_command.sql create mode 100644 metadata/migrations/0039_add_agent_command_event_type.sql diff --git a/agent/sentry-agent/src/main.rs b/agent/sentry-agent/src/main.rs index eecf41b..4e10c34 100644 --- a/agent/sentry-agent/src/main.rs +++ b/agent/sentry-agent/src/main.rs @@ -20,7 +20,7 @@ use anyhow::{Context, Result}; use batch::Batcher; use clap::Parser; use config::Config; -use pb::agent::v1::{agent_control_client::AgentControlClient, CheckInRequest, DesiredOverride, ReportedConfig}; +use pb::agent::v1::{agent_control_client::AgentControlClient, AgentCommand, CheckInRequest, DesiredOverride, ReportedConfig}; use pb::{log_ingest_client::LogIngestClient, LogRecord, Severity}; use std::path::PathBuf; use std::time::Duration; @@ -171,6 +171,26 @@ pub async fn run_agent(config_path: Option) -> Result<()> { tracing::info!(version = %applied_override_version, "applied remote config override"); } } + if AgentCommand::try_from(resp.pending_command) == Ok(AgentCommand::Restart) { + tracing::info!("received remote restart command, shutting down gracefully"); + // flush_all(), not poll_timeout(): same reasoning as + // the normal shutdown path below -- whatever's + // buffered must go out regardless of whether + // flush_interval has elapsed yet. + if let Some(batch) = batcher.flush_all() { + flush(&mut client, batch).await; + } + source_handle.abort(); + // A hard process exit, not a `break` out of this + // loop: this agent's own restart policy is + // entirely the host's service manager's + // responsibility (systemd/Windows SCM), the same + // contract any well-behaved service relies on -- + // see AgentCommand's doc comment for why STOP/ + // UNINSTALL need real per-platform work this + // doesn't. + std::process::exit(0); + } } // A failed check-in is not fatal -- same graceful- // degradation posture as a failed heartbeat/batch diff --git a/api/agents/handler.go b/api/agents/handler.go index b630515..ee46725 100644 --- a/api/agents/handler.go +++ b/api/agents/handler.go @@ -18,28 +18,58 @@ type store interface { Get(ctx context.Context, tenantID, host string) (*Agent, error) SetOverride(ctx context.Context, tenantID, host string, override ConfigOverride, updatedBy string) (*Agent, error) ClearOverride(ctx context.Context, tenantID, host string) error + IssueCommand(ctx context.Context, tenantID, host, command, issuedBy string) (*Agent, error) +} + +// CommandLogger records an issued lifecycle command into the Phase 4 +// audit trail -- same nil-by-default, fail-open shape as +// aiapi.InteractionLogger and queryapi.AuditLogger: a single-tenant +// deployment with no enterprise/ configured just doesn't log these. +// enterprise/internal/audit supplies the real implementation +// (event_type = 'agent_command', see +// metadata/migrations/0039_add_agent_command_event_type.sql) -- this is +// the one entry point in this package genuinely worth logging even +// without enterprise/ wired, given "strict RBAC, full audit trail" was +// the explicit precondition for building lifecycle commands at all (see +// /docs/agent-management-design.md); it degrades gracefully rather than +// being required, matching every other optional audit hook in this +// codebase, but a real deployment should wire it. +type CommandLogger interface { + LogCommand(ctx context.Context, entry CommandLogEntry) error +} + +type CommandLogEntry struct { + Host string + Command string + IssuedBy string } type Handler struct { logger *slog.Logger store store authorizer authz.Authorizer + commands CommandLogger } -func NewHandler(logger *slog.Logger, store store, authorizer authz.Authorizer) *Handler { - return &Handler{logger: logger, store: store, authorizer: authorizer} +// commands may be nil -- see CommandLogger's doc comment. +func NewHandler(logger *slog.Logger, store store, authorizer authz.Authorizer, commands CommandLogger) *Handler { + return &Handler{logger: logger, store: store, authorizer: authorizer, commands: commands} } // RegisterRoutes: viewing inventory is RoleViewer (same bar as viewing // a dashboard); editing an agent's remote config is RoleEditor -- an // operational-tuning action, not an admin-only one, matching the RBAC // matrix's treatment of alert rules/notification targets rather than -// user/role management. +// user/role management. Issuing a lifecycle command is RoleAdmin -- +// stricter than config editing, matching the matrix's treatment of +// similarly consequential actions (e.g. deleting a notification +// target) rather than day-to-day tuning. func (h *Handler) RegisterRoutes(mux *http.ServeMux) { mux.HandleFunc("GET /agents", authz.RequireRole(h.authorizer, authz.RoleViewer, h.handleList)) mux.HandleFunc("GET /agents/{host}", authz.RequireRole(h.authorizer, authz.RoleViewer, h.handleGet)) mux.HandleFunc("PUT /agents/{host}/config", authz.RequireRole(h.authorizer, authz.RoleEditor, h.handleSetConfig)) mux.HandleFunc("DELETE /agents/{host}/config", authz.RequireRole(h.authorizer, authz.RoleEditor, h.handleClearConfig)) + mux.HandleFunc("PUT /agents/{host}/command", authz.RequireRole(h.authorizer, authz.RoleAdmin, h.handleIssueCommand)) } // tenantID mirrors dashboards.Handler.tenantID exactly -- resolved from @@ -106,6 +136,43 @@ func (h *Handler) handleSetConfig(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, a) } +type issueCommandRequest struct { + Command string `json:"command"` +} + +// handleIssueCommand queues a one-shot lifecycle command -- see +// Store.IssueCommand's doc comment for the delivery/clearing semantics. +// Logs to CommandLogger fail-open (a write failure is logged server- +// side and otherwise ignored, same posture as aiapi's interaction +// logging): audit-trail completeness matters, but it shouldn't be able +// to turn a legitimate restart request into a 500. +func (h *Handler) handleIssueCommand(w http.ResponseWriter, r *http.Request) { + var req issueCommandRequest + if !decodeJSON(w, r, &req) { + return + } + if !validCommand(req.Command) { + writeError(w, http.StatusBadRequest, `command must be "restart"`) + return + } + + host := r.PathValue("host") + issuedBy := h.updatedBy(r) + a, err := h.store.IssueCommand(r.Context(), h.tenantID(r), host, req.Command, issuedBy) + if err != nil { + h.writeStoreErr(w, err, "issuing agent command") + return + } + + if h.commands != nil { + if err := h.commands.LogCommand(r.Context(), CommandLogEntry{Host: host, Command: req.Command, IssuedBy: issuedBy}); err != nil { + h.logger.Error("logging agent command to audit trail", "host", host, "command", req.Command, "error", err) + } + } + + writeJSON(w, http.StatusOK, a) +} + func (h *Handler) handleClearConfig(w http.ResponseWriter, r *http.Request) { if err := h.store.ClearOverride(r.Context(), h.tenantID(r), r.PathValue("host")); err != nil { h.writeStoreErr(w, err, "clearing agent config") diff --git a/api/agents/handler_test.go b/api/agents/handler_test.go index 44ca76d..9d86272 100644 --- a/api/agents/handler_test.go +++ b/api/agents/handler_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "io" "log/slog" "net/http" @@ -78,8 +79,33 @@ func (f *fakeStore) ClearOverride(_ context.Context, tenantID, host string) erro return nil } +func (f *fakeStore) IssueCommand(_ context.Context, tenantID, host, command, issuedBy string) (*Agent, error) { + a, ok := f.agents[tenantID+"/"+host] + if !ok { + return nil, ErrNotFound + } + a.PendingCommand = command + a.CommandIssuedBy = issuedBy + cp := *a + return &cp, nil +} + +// fakeCommandLogger records LogCommand calls for assertions; nil-safe +// callers should use a nil *fakeCommandLogger the same way production +// code treats a nil CommandLogger, but tests that want to assert +// logging happened construct a real one. +type fakeCommandLogger struct { + entries []CommandLogEntry + err error +} + +func (f *fakeCommandLogger) LogCommand(_ context.Context, entry CommandLogEntry) error { + f.entries = append(f.entries, entry) + return f.err +} + func newTestHandler(s *fakeStore) *Handler { - return NewHandler(discardLogger(), s, nil) + return NewHandler(discardLogger(), s, nil, nil) } func doRequest(t *testing.T, h *Handler, method, path string, body any) *httptest.ResponseRecorder { @@ -200,7 +226,7 @@ func TestRequireEditorRoleForConfigWrites(t *testing.T) { s := newFakeStore() s.put(Agent{TenantID: "default", Host: "web-01"}) authorizer := fakeAuthorizer{role: authz.RoleViewer} - h := NewHandler(discardLogger(), s, authorizer) + h := NewHandler(discardLogger(), s, authorizer, nil) interval := int64(30000) rec := doRequest(t, h, "PUT", "/agents/web-01/config", ConfigOverride{HeartbeatIntervalMS: &interval}) @@ -209,6 +235,75 @@ func TestRequireEditorRoleForConfigWrites(t *testing.T) { } } +func TestHandleIssueCommandRoundTrips(t *testing.T) { + s := newFakeStore() + s.put(Agent{TenantID: "default", Host: "web-01"}) + logger := &fakeCommandLogger{} + h := NewHandler(discardLogger(), s, nil, logger) + + rec := doRequest(t, h, "PUT", "/agents/web-01/command", map[string]string{"command": "restart"}) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String()) + } + var got Agent + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("decoding response: %v", err) + } + if got.PendingCommand != "restart" { + t.Fatalf("PendingCommand = %q, want restart", got.PendingCommand) + } + if len(logger.entries) != 1 || logger.entries[0].Command != "restart" || logger.entries[0].Host != "web-01" { + t.Fatalf("unexpected audit log entries: %+v", logger.entries) + } +} + +func TestHandleIssueCommandRejectsUnknownCommand(t *testing.T) { + s := newFakeStore() + s.put(Agent{TenantID: "default", Host: "web-01"}) + h := newTestHandler(s) + + rec := doRequest(t, h, "PUT", "/agents/web-01/command", map[string]string{"command": "uninstall"}) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (uninstall is not a supported command yet)", rec.Code) + } +} + +func TestHandleIssueCommandUnknownHostIsNotFound(t *testing.T) { + h := newTestHandler(newFakeStore()) + rec := doRequest(t, h, "PUT", "/agents/nope/command", map[string]string{"command": "restart"}) + if rec.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404", rec.Code) + } +} + +// TestHandleIssueCommandFailOpenOnLoggerError is the regression test +// for CommandLogger's documented fail-open posture: an audit-log write +// failure must not turn a legitimate command issuance into an error +// response. +func TestHandleIssueCommandFailOpenOnLoggerError(t *testing.T) { + s := newFakeStore() + s.put(Agent{TenantID: "default", Host: "web-01"}) + logger := &fakeCommandLogger{err: errors.New("audit db unreachable")} + h := NewHandler(discardLogger(), s, nil, logger) + + rec := doRequest(t, h, "PUT", "/agents/web-01/command", map[string]string{"command": "restart"}) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 even though the audit logger failed", rec.Code) + } +} + +func TestRequireAdminRoleForCommands(t *testing.T) { + s := newFakeStore() + s.put(Agent{TenantID: "default", Host: "web-01"}) + authorizer := fakeAuthorizer{role: authz.RoleEditor} + h := NewHandler(discardLogger(), s, authorizer, nil) + + rec := doRequest(t, h, "PUT", "/agents/web-01/command", map[string]string{"command": "restart"}) + if rec.Code != http.StatusForbidden { + t.Fatalf("status = %d, want 403 (Editor must not be able to issue lifecycle commands, only Admin+)", rec.Code) + } +} + type fakeAuthorizer struct { role authz.Role } diff --git a/api/agents/store.go b/api/agents/store.go index 4e95403..a74de94 100644 --- a/api/agents/store.go +++ b/api/agents/store.go @@ -17,6 +17,16 @@ import ( var ErrNotFound = errors.New("not found") +// CommandRestart is the one supported lifecycle command -- see +// ingest/internal/grpcserver.AgentCommandRestart and +// agent_control.proto's AgentCommand enum comment for why STOP/ +// UNINSTALL aren't here yet. +const CommandRestart = "restart" + +func validCommand(c string) bool { + return c == CommandRestart +} + // ConfigOverride is the remotely-editable subset of an agent's config -- // a plain-Go mirror of ingest/internal/agentregistry's overrideFields // and agent_control.proto's DesiredOverride. Deliberately duplicated @@ -57,6 +67,17 @@ type Agent struct { // recomputing the same string comparison itself. Pending bool `json:"pending"` UpdatedBy string `json:"updated_by,omitempty"` + // PendingCommand is "" when nothing is queued, or CommandRestart + // while a restart hasn't yet been delivered to the agent. Unlike + // Pending (config), there's no way to observe "delivered" from this + // table alone -- ingest clears pending_command the instant it hands + // the command out (see ingest/internal/agentregistry.Registry. + // CheckIn), so PendingCommand flipping back to "" just as plausibly + // means "delivered a moment ago" as "never issued." CommandIssuedAt + // is what the web UI shows instead, as a last-issued record. + PendingCommand string `json:"pending_command,omitempty"` + CommandIssuedAt *time.Time `json:"command_issued_at,omitempty"` + CommandIssuedBy string `json:"command_issued_by,omitempty"` } type Store struct { @@ -73,7 +94,8 @@ const selectColumns = ` reported_batch_max_size, reported_batch_flush_ms, reported_heartbeat_on, reported_heartbeat_ms, first_seen_at, last_seen_at, - desired_override, desired_override_version, applied_override_version, updated_by` + desired_override, desired_override_version, applied_override_version, updated_by, + pending_command, command_issued_at, command_issued_by` func (s *Store) List(ctx context.Context, tenantID string) ([]Agent, error) { rows, err := s.pool.Query(ctx, ` @@ -145,6 +167,27 @@ func (s *Store) SetOverride(ctx context.Context, tenantID, host string, override return s.Get(ctx, tenantID, host) } +// IssueCommand queues a one-shot lifecycle command for host, delivered +// on its next CheckIn and cleared atomically by ingest the instant +// that happens (see ingest/internal/agentregistry.Registry.CheckIn) -- +// unlike SetOverride, there's no "applied" confirmation to wait for, +// since a restarting agent's process is gone before it could send one. +// command_issued_at/by are overwritten on every call, forming a +// last-issued record even after pending_command itself clears. +func (s *Store) IssueCommand(ctx context.Context, tenantID, host, command, issuedBy string) (*Agent, error) { + tag, err := s.pool.Exec(ctx, ` + UPDATE agents SET pending_command = $1, command_issued_at = now(), command_issued_by = $2 + WHERE tenant_id = $3 AND host = $4`, + command, issuedBy, tenantID, host) + if err != nil { + return nil, err + } + if tag.RowsAffected() == 0 { + return nil, ErrNotFound + } + return s.Get(ctx, tenantID, host) +} + // ClearOverride reverts an agent to running its local agent.toml // untouched -- the next CheckIn gets has_override=false. func (s *Store) ClearOverride(ctx context.Context, tenantID, host string) error { @@ -167,7 +210,7 @@ type rowScanner interface { func scanAgent(row rowScanner) (Agent, error) { var a Agent var desiredOverride []byte - var desiredVersion, updatedBy *string + var desiredVersion, updatedBy, pendingCommand, commandIssuedBy *string if err := row.Scan( &a.ID, &a.TenantID, &a.Host, &a.Service, &a.AgentVersion, &a.SourceKind, &a.SourceDetail, @@ -175,6 +218,7 @@ func scanAgent(row rowScanner) (Agent, error) { &a.HeartbeatEnabled, &a.HeartbeatIntervalMS, &a.FirstSeenAt, &a.LastSeenAt, &desiredOverride, &desiredVersion, &a.AppliedOverrideVersion, &updatedBy, + &pendingCommand, &a.CommandIssuedAt, &commandIssuedBy, ); err != nil { if errors.Is(err, pgx.ErrNoRows) { return Agent{}, ErrNotFound @@ -184,6 +228,12 @@ func scanAgent(row rowScanner) (Agent, error) { if updatedBy != nil { a.UpdatedBy = *updatedBy } + if pendingCommand != nil { + a.PendingCommand = *pendingCommand + } + if commandIssuedBy != nil { + a.CommandIssuedBy = *commandIssuedBy + } if desiredVersion != nil { a.DesiredOverrideVersion = *desiredVersion a.Pending = *desiredVersion != a.AppliedOverrideVersion diff --git a/api/cmd/api/main.go b/api/cmd/api/main.go index 222e1cb..1abe79d 100644 --- a/api/cmd/api/main.go +++ b/api/cmd/api/main.go @@ -126,7 +126,10 @@ func main() { // AGENT_REGISTRY_POSTGRES_ADDR to be set on ingest; these routes work // unconditionally, they'll just show an empty inventory if ingest // hasn't been configured to record check-ins. - agentsHandler := agents.NewHandler(logger, agents.NewStore(pgPool), authorizer) + // nil command logger: core has no enterprise/internal/audit + // implementation to log lifecycle commands against, same posture as + // queryHandler's/aiHandler's nil audit loggers above. + agentsHandler := agents.NewHandler(logger, agents.NewStore(pgPool), authorizer, nil) // One shared mux, CORS applied once around the whole thing -- see // httpserver's doc comment for why this changed from each diff --git a/docs/agent-management-design.md b/docs/agent-management-design.md index 2baaf26..3742b0d 100644 --- a/docs/agent-management-design.md +++ b/docs/agent-management-design.md @@ -13,9 +13,16 @@ 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. +own security design (strict RBAC, full audit trail) before it's built, +not something to fold in as a side effect of a config-editing feature. + +**Update — restart is now built** (punch-list item 1, see the +"Lifecycle commands" section below); stop and uninstall remain +deliberately out of scope, for reasons specific to each. Building +restart confirmed the "strict RBAC, full audit trail" precondition +mattered in practice, not just as a stated principle: it's gated at +`RoleAdmin` (stricter than config editing's `RoleEditor`) and logged +into the same `audit_log` table Phase 7's AI interactions use. ## Why this is still pull, not push, on the wire @@ -167,16 +174,84 @@ together. Keep the three in sync by hand. 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. +not an admin-only capability like user/role management. Issuing a +lifecycle command is `RoleAdmin` — stricter, matching the RBAC matrix's +treatment of similarly consequential actions (e.g. deleting a +notification target) rather than day-to-day tuning. + +## Lifecycle commands (punch-list item 1: restart) + +A one-shot action, not a persistent desired state like `DesiredOverride` +-- `AgentCommand` (proto enum, `agent_control.proto`) is delivered +**at-most-once**: `ingest/internal/agentregistry.Registry.CheckIn` +clears `agents.pending_command` in the same transaction that hands it +to the agent, before the agent has any chance to confirm it executed +the command. This is a deliberate, disclosed asymmetry from +`DesiredOverride`'s "keep re-offering until the agent's +`applied_override_version` matches" semantics: a restarting agent's +process is gone before it could ever send that confirmation, so waiting +for one isn't possible. A command lost to a network blip between the +response and the agent acting on it is simply lost -- re-issuing (`PUT +/agents/{host}/command` again) is the operator's recourse, the same as +it would be for a `systemctl restart` that silently failed to reach its +target. + +Only `restart` exists today. `stop` and `uninstall` remain deliberately +deferred -- both need real OS service-manager integration (systemd's +`Restart=`/`RestartPreventExitStatus=` semantics vs. Windows SCM +recovery options differ enough per platform that hand-waving them would +be dishonest), which `restart` doesn't: the agent does a graceful +shutdown (flushing whatever's buffered via `Batcher::flush_all`, +aborting the source task) and then a clean `std::process::exit(0)`, +relying entirely on whatever restart policy the host's service manager +already has configured -- the same contract any well-behaved service +already expects. `api/agents.Store.IssueCommand`/the `agents` table's +`pending_command` column are written generically enough that adding +`stop`/`uninstall` later is a real per-platform agent-side +implementation, not a data-model change. + +Logged into the same append-only `audit_log` table as everything else +privileged in this codebase (`event_type = 'agent_command'`, +`metadata/migrations/0039`), via `enterprise/internal/audit. +AgentCommandLogger` (`api/agents.CommandLogger`, nil-by-default in core +same as every other optional audit hook) -- fail-open, same posture as +Phase 7's AI-interaction logging: a write failure is logged server-side +and never turns a legitimate restart into a 500. + +**A real bug was found and fixed while verifying this live.** The +first version of `agentregistry.Registry.CheckIn` tried to read-and- +clear `pending_command` atomically using a single `INSERT ... ON +CONFLICT` statement with a sibling read-only CTE referenced only from +`RETURNING`, on the assumption that Postgres evaluates every part of a +`WITH` query against the same pre-statement snapshot. That assumption +is wrong specifically for `FOR UPDATE`: it always locks (and therefore +reads) the *latest* row version to do its job, including a version +written earlier in the very same statement -- confirmed empirically +against a live Postgres (the CTE's `FOR UPDATE` was reading its own +sibling `UPDATE`'s just-cleared `NULL`, so `pending_command` always +came back empty even when a command was genuinely pending, and the +agent never received it). Live verification caught this immediately: a +restart command showed `pending_command: "restart"` in the API +response, but the agent never logged receiving it and never exited. +Fixed by splitting into two real, ordered statements inside one +explicit transaction (`SELECT ... FOR UPDATE` strictly happens-before +the clearing `UPDATE`) -- unambiguous, no snapshot-timing subtlety to +get wrong. ## 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. +A real agent binary, its heartbeat/CheckIn cadence pointed at a live +`ingest` with `AGENT_REGISTRY_POSTGRES_ADDR` configured, confirming: the +agent appears in `GET /agents` after its first check-in; an edit made +via `PUT /agents/{host}/config` shows `pending: true` immediately and +`pending: false` after the agent's next check-in; 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; a journald-unit override triggers a real source-task +restart, reflected in the next reported `source_detail`; and (after the +bug above was fixed) a restart command issued via `PUT /agents/{host}/ +command` is picked up on the agent's very next check-in, logged +(`"received remote restart command, shutting down gracefully"`), and +the process exits cleanly -- `pending_command` confirmed cleared and +`ps` confirming the process gone. + diff --git a/enterprise/cmd/enterprise-api/main.go b/enterprise/cmd/enterprise-api/main.go index 2a3173a..08536e2 100644 --- a/enterprise/cmd/enterprise-api/main.go +++ b/enterprise/cmd/enterprise-api/main.go @@ -165,7 +165,11 @@ func main() { queryHandler := queryapi.NewHandler(logger, registry, search, cfg.QueryTimeout, auditLogger, authorizer) dashboardsHandler := dashboards.NewHandler(logger, dashboards.NewStore(pgPool), authorizer, rbacstore.NewDashboardPermissions(rbac)) - agentsHandler := agents.NewHandler(logger, agents.NewStore(pgPool), authorizer) + // Reuses the same audit_writer-role pool/store auditLogger above + // writes through -- same append-only audit_log table, new + // event_type (see metadata/migrations/0039). + commandLogger := audit.NewAgentCommandLogger(audit.NewStore(auditPool), audit.SourceAPI) + agentsHandler := agents.NewHandler(logger, agents.NewStore(pgPool), authorizer, commandLogger) mux := http.NewServeMux() queryHandler.RegisterRoutes(mux) // also registers GET /healthz diff --git a/enterprise/internal/audit/agent_command_adapter.go b/enterprise/internal/audit/agent_command_adapter.go new file mode 100644 index 0000000..fe74f8c --- /dev/null +++ b/enterprise/internal/audit/agent_command_adapter.go @@ -0,0 +1,59 @@ +// Adapts *Store to api/agents.CommandLogger -- same shape as +// ai_interaction_adapter.go's AIInteractionLogger, wired in by +// enterprise/cmd/enterprise-api alongside it. +package audit + +import ( + "context" + "encoding/json" + "fmt" + + "github.com/sentry/sentry/api/agents" + "github.com/sentry/sentry/api/authz" +) + +// AgentCommandLogger implements agents.CommandLogger by translating its +// CommandLogEntry into this package's Entry, reading tenant/user +// identity from ctx -- same "read identity from ctx rather than the +// interface growing tenant-awareness" shape as every other adapter in +// this package. +type AgentCommandLogger struct { + store *Store + source Source +} + +func NewAgentCommandLogger(store *Store, source Source) *AgentCommandLogger { + return &AgentCommandLogger{store: store, source: source} +} + +type agentCommandDetail struct { + Host string `json:"host"` + Command string `json:"command"` +} + +func (l *AgentCommandLogger) LogCommand(ctx context.Context, entry agents.CommandLogEntry) error { + identity, ok := authz.IdentityFromContext(ctx) + if !ok || identity.TenantID == "" { + return fmt.Errorf("audit: no tenant identity in context, refusing to write an unattributable audit entry") + } + + var userID *string + if identity.UserID != "" { + userID = &identity.UserID + } + + detail, err := json.Marshal(agentCommandDetail{Host: entry.Host, Command: entry.Command}) + if err != nil { + return fmt.Errorf("audit: marshaling agent command detail: %w", err) + } + + _, err = l.store.Append(ctx, Entry{ + TenantID: identity.TenantID, + UserID: userID, + Source: l.source, + EventType: EventAgentCommand, + Status: StatusSuccess, + Detail: detail, + }) + return err +} diff --git a/enterprise/internal/audit/audit.go b/enterprise/internal/audit/audit.go index ce662b3..7e74629 100644 --- a/enterprise/internal/audit/audit.go +++ b/enterprise/internal/audit/audit.go @@ -53,6 +53,14 @@ const ( // original input, confidence, and whether the user edited the // suggestion before using it -- see ai_interaction_adapter.go. EventAIInteraction EventType = "ai_interaction" + // EventAgentCommand: a lifecycle command (restart) issued to an + // agent. Detail carries the target host and the command; Status is + // always success here -- this logs that the command was *issued* to + // storage, not that the agent confirmed executing it (a restarting + // agent's process can't send that confirmation -- see + // agent_control.proto's CheckInResponse comment). See + // agent_command_adapter.go. + EventAgentCommand EventType = "agent_command" ) type Status string diff --git a/ingest/internal/agentregistry/agentregistry.go b/ingest/internal/agentregistry/agentregistry.go index c3b4724..cde0a18 100644 --- a/ingest/internal/agentregistry/agentregistry.go +++ b/ingest/internal/agentregistry/agentregistry.go @@ -10,9 +10,11 @@ package agentregistry import ( "context" "encoding/json" + "errors" "fmt" "github.com/google/uuid" + "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/sentry/sentry/ingest/internal/grpcserver" @@ -49,16 +51,47 @@ func New(pool *pgxpool.Pool) *Registry { var _ grpcserver.AgentRegistry = (*Registry)(nil) -func (r *Registry) CheckIn(ctx context.Context, tenantID string, info grpcserver.AgentCheckIn) (grpcserver.AgentOverride, error) { +func (r *Registry) CheckIn(ctx context.Context, tenantID string, info grpcserver.AgentCheckIn) (grpcserver.CheckInResult, error) { if tenantID == "" { tenantID = defaultTenantID } + tx, err := r.pool.Begin(ctx) + if err != nil { + return grpcserver.CheckInResult{}, fmt.Errorf("agentregistry: beginning transaction: %w", err) + } + defer tx.Rollback(ctx) + + // Read (and lock) whatever pending_command exists BEFORE the + // upsert below clears it -- as two real, ordered statements in one + // transaction, not one clever statement. An earlier version tried + // to do this with a single INSERT...ON CONFLICT plus a sibling CTE + // referenced only from RETURNING, on the assumption that Postgres + // evaluates every part of a WITH query against the same pre- + // statement snapshot; that assumption is wrong specifically for + // FOR UPDATE, which always locks (and therefore reads) the latest + // row version to do its job, including versions written earlier in + // the SAME statement -- confirmed empirically against a live + // Postgres (the CTE's FOR UPDATE was reading its own sibling + // UPDATE's just-cleared NULL, always reporting "no command" even + // when one was genuinely pending). Two statements in one + // transaction has no such ambiguity: the SELECT strictly + // happens-before the UPDATE, full stop. FOR UPDATE against zero + // rows (a brand-new agent's first-ever check-in) is a harmless + // no-op -- there's nothing to lock or have a pending command yet. + var pendingCommand *string + err = tx.QueryRow(ctx, `SELECT pending_command FROM agents WHERE tenant_id = $1 AND host = $2 FOR UPDATE`, + tenantID, info.Host, + ).Scan(&pendingCommand) + if err != nil && !errors.Is(err, pgx.ErrNoRows) { + return grpcserver.CheckInResult{}, fmt.Errorf("agentregistry: reading pending command: %w", err) + } + var ( desiredOverride []byte desiredVersion *string ) - err := r.pool.QueryRow(ctx, ` + err = tx.QueryRow(ctx, ` INSERT INTO agents ( id, tenant_id, host, service, reported_agent_version, reported_source_kind, reported_source_detail, @@ -76,7 +109,8 @@ func (r *Registry) CheckIn(ctx context.Context, tenantID string, info grpcserver reported_heartbeat_on = EXCLUDED.reported_heartbeat_on, reported_heartbeat_ms = EXCLUDED.reported_heartbeat_ms, last_seen_at = now(), - applied_override_version = EXCLUDED.applied_override_version + applied_override_version = EXCLUDED.applied_override_version, + pending_command = NULL RETURNING desired_override, desired_override_version`, uuid.NewString(), tenantID, info.Host, info.Service, info.AgentVersion, info.SourceKind, info.SourceDetail, @@ -85,18 +119,27 @@ func (r *Registry) CheckIn(ctx context.Context, tenantID string, info grpcserver info.AppliedOverrideVersion, ).Scan(&desiredOverride, &desiredVersion) if err != nil { - return grpcserver.AgentOverride{}, fmt.Errorf("agentregistry: upserting check-in: %w", err) + return grpcserver.CheckInResult{}, fmt.Errorf("agentregistry: upserting check-in: %w", err) + } + + if err := tx.Commit(ctx); err != nil { + return grpcserver.CheckInResult{}, fmt.Errorf("agentregistry: committing check-in: %w", err) + } + + result := grpcserver.CheckInResult{} + if pendingCommand != nil { + result.Command = *pendingCommand } if desiredVersion == nil || len(desiredOverride) == 0 { - return grpcserver.AgentOverride{HasOverride: false}, nil + return result, nil } var fields overrideFields if err := json.Unmarshal(desiredOverride, &fields); err != nil { - return grpcserver.AgentOverride{}, fmt.Errorf("agentregistry: parsing stored override: %w", err) + return grpcserver.CheckInResult{}, fmt.Errorf("agentregistry: parsing stored override: %w", err) } - return grpcserver.AgentOverride{ + result.Override = grpcserver.AgentOverride{ HasOverride: true, BatchMaxSize: fields.BatchMaxSize, BatchFlushIntervalMS: fields.BatchFlushIntervalMS, @@ -104,5 +147,6 @@ func (r *Registry) CheckIn(ctx context.Context, tenantID string, info grpcserver HeartbeatIntervalMS: fields.HeartbeatIntervalMS, JournaldUnit: fields.JournaldUnit, Version: *desiredVersion, - }, nil + } + return result, nil } diff --git a/ingest/internal/grpcserver/server.go b/ingest/internal/grpcserver/server.go index a00af08..2e200d6 100644 --- a/ingest/internal/grpcserver/server.go +++ b/ingest/internal/grpcserver/server.go @@ -91,9 +91,32 @@ type TenantResolver interface { // simply doesn't get agent inventory/management, exactly like one // without ENTERPRISE_AUTH_URL doesn't get tenant-tagged records. type AgentRegistry interface { - CheckIn(ctx context.Context, tenantID string, info AgentCheckIn) (AgentOverride, error) + CheckIn(ctx context.Context, tenantID string, info AgentCheckIn) (CheckInResult, error) } +// CheckInResult bundles the two independent things a CheckIn can hand +// back to an agent -- a persistent config override to converge to, and +// a one-shot command to act on immediately. Kept as two separate +// concepts (not folded into one "override" shape) since their delivery +// semantics differ: Override is re-offered every CheckIn until the +// agent's applied_override_version matches; Command is cleared the +// instant it's handed out (see agent_control.proto's CheckInResponse +// comment). +type CheckInResult struct { + Override AgentOverride + // Command is AgentCommandRestart or "" (nothing pending). A plain + // string, not the generated proto enum type, so AgentRegistry + // implementations don't need to import the gRPC-facing package -- + // same reasoning as AgentCheckIn/AgentOverride below. + Command string +} + +// AgentCommandRestart is the one supported value for +// CheckInResult.Command / the agents table's pending_command column -- +// see agent_control.proto's AgentCommand enum comment for why STOP/ +// UNINSTALL aren't here yet. +const AgentCommandRestart = "restart" + // AgentCheckIn is what an agent reports about itself on each CheckIn -- // a plain-Go mirror of agentv1.ReportedConfig plus the identity/ // tenant fields, kept separate from the proto type so AgentRegistry @@ -250,7 +273,7 @@ func (s *Server) CheckIn(ctx context.Context, req *agentv1.CheckInRequest) (*age } cfg := req.GetCurrentConfig() - override, err := s.agents.CheckIn(ctx, tenantID, AgentCheckIn{ + result, err := s.agents.CheckIn(ctx, tenantID, AgentCheckIn{ Host: req.GetHost(), Service: req.GetService(), AgentVersion: cfg.GetAgentVersion(), @@ -267,20 +290,27 @@ func (s *Server) CheckIn(ctx context.Context, req *agentv1.CheckInRequest) (*age return nil, status.Errorf(codes.Internal, "recording check-in: %v", err) } - if !override.HasOverride { - return &agentv1.CheckInResponse{HasOverride: false}, nil + resp := &agentv1.CheckInResponse{HasOverride: result.Override.HasOverride} + if result.Override.HasOverride { + resp.Override = &agentv1.DesiredOverride{ + BatchMaxSize: result.Override.BatchMaxSize, + BatchFlushIntervalMs: result.Override.BatchFlushIntervalMS, + HeartbeatEnabled: result.Override.HeartbeatEnabled, + HeartbeatIntervalMs: result.Override.HeartbeatIntervalMS, + JournaldUnit: result.Override.JournaldUnit, + Version: result.Override.Version, + } } - return &agentv1.CheckInResponse{ - HasOverride: true, - Override: &agentv1.DesiredOverride{ - BatchMaxSize: override.BatchMaxSize, - BatchFlushIntervalMs: override.BatchFlushIntervalMS, - HeartbeatEnabled: override.HeartbeatEnabled, - HeartbeatIntervalMs: override.HeartbeatIntervalMS, - JournaldUnit: override.JournaldUnit, - Version: override.Version, - }, - }, nil + switch result.Command { + case AgentCommandRestart: + resp.PendingCommand = agentv1.AgentCommand_AGENT_COMMAND_RESTART + s.logger.Info("delivering restart command to agent", "host", req.GetHost()) + case "": + // nothing pending + default: + s.logger.Error("agent registry returned an unknown command, ignoring", "host", req.GetHost(), "command", result.Command) + } + return resp, nil } // bearerTokenFromContext reads the same "authorization: Bearer " diff --git a/ingest/internal/grpcserver/server_test.go b/ingest/internal/grpcserver/server_test.go index a9a04f8..7232848 100644 --- a/ingest/internal/grpcserver/server_test.go +++ b/ingest/internal/grpcserver/server_test.go @@ -59,20 +59,23 @@ type fakeAgentRegistry struct { mu sync.Mutex checkIns []AgentCheckIn overrides map[string]AgentOverride + commands map[string]string err error } -func (f *fakeAgentRegistry) CheckIn(_ context.Context, tenantID string, info AgentCheckIn) (AgentOverride, error) { +func (f *fakeAgentRegistry) CheckIn(_ context.Context, tenantID string, info AgentCheckIn) (CheckInResult, error) { f.mu.Lock() defer f.mu.Unlock() if f.err != nil { - return AgentOverride{}, f.err + return CheckInResult{}, f.err } f.checkIns = append(f.checkIns, info) - if f.overrides == nil { - return AgentOverride{HasOverride: false}, nil + key := tenantID + "/" + info.Host + result := CheckInResult{Command: f.commands[key]} + if f.overrides != nil { + result.Override = f.overrides[key] } - return f.overrides[tenantID+"/"+info.Host], nil + return result, nil } func newTestServer(p batchProducer) *Server { @@ -374,3 +377,29 @@ func TestCheckInWithResolverRejectsMissingToken(t *testing.T) { t.Fatalf("CheckIn() error = %v, want Unauthenticated", err) } } + +func TestCheckInDeliversPendingRestartCommand(t *testing.T) { + reg := &fakeAgentRegistry{commands: map[string]string{"/web-01": AgentCommandRestart}} + s := newTestServerWithAgents(reg) + + resp, err := s.CheckIn(context.Background(), &agentv1.CheckInRequest{Host: "web-01", CurrentConfig: &agentv1.ReportedConfig{}}) + if err != nil { + t.Fatalf("CheckIn() error = %v", err) + } + if resp.GetPendingCommand() != agentv1.AgentCommand_AGENT_COMMAND_RESTART { + t.Fatalf("PendingCommand = %v, want AGENT_COMMAND_RESTART", resp.GetPendingCommand()) + } +} + +func TestCheckInNoCommandReportsUnspecified(t *testing.T) { + reg := &fakeAgentRegistry{} + s := newTestServerWithAgents(reg) + + resp, err := s.CheckIn(context.Background(), &agentv1.CheckInRequest{Host: "web-01", CurrentConfig: &agentv1.ReportedConfig{}}) + if err != nil { + t.Fatalf("CheckIn() error = %v", err) + } + if resp.GetPendingCommand() != agentv1.AgentCommand_AGENT_COMMAND_UNSPECIFIED { + t.Fatalf("PendingCommand = %v, want AGENT_COMMAND_UNSPECIFIED", resp.GetPendingCommand()) + } +} diff --git a/metadata/migrations/0038_add_agent_pending_command.sql b/metadata/migrations/0038_add_agent_pending_command.sql new file mode 100644 index 0000000..e2645f0 --- /dev/null +++ b/metadata/migrations/0038_add_agent_pending_command.sql @@ -0,0 +1,18 @@ +-- Agent lifecycle commands (Phase: agent management punch-list item 1, +-- see /docs/agent-management-design.md). A one-shot action, not a +-- persistent desired state like desired_override -- pending_command is +-- cleared atomically by ingest/internal/agentregistry.Registry.CheckIn +-- the moment it's handed to the agent in a response, not once the agent +-- confirms execution (a restarting agent's process is gone before it +-- could send that confirmation). issued_at/issued_by are kept even +-- after the command clears, as the last-issued record for the web UI +-- and as a lightweight trail alongside the real audit_log entry +-- (event_type = 'agent_command', see enterprise/internal/audit). +ALTER TABLE agents + ADD COLUMN pending_command TEXT, + ADD COLUMN command_issued_at TIMESTAMPTZ, + ADD COLUMN command_issued_by TEXT; + +ALTER TABLE agents + ADD CONSTRAINT agents_pending_command_check + CHECK (pending_command IS NULL OR pending_command IN ('restart')); diff --git a/metadata/migrations/0039_add_agent_command_event_type.sql b/metadata/migrations/0039_add_agent_command_event_type.sql new file mode 100644 index 0000000..728190c --- /dev/null +++ b/metadata/migrations/0039_add_agent_command_event_type.sql @@ -0,0 +1,9 @@ +-- Agent lifecycle commands are audited through the same append-only, +-- hash-chained audit_log table every other privileged action already +-- uses -- same extension shape as 0036_add_ai_interaction_event_type.sql. +-- Postgres has no ALTER CHECK, so drop and recreate. +ALTER TABLE audit_log DROP CONSTRAINT audit_log_event_type_check; + +ALTER TABLE audit_log + ADD CONSTRAINT audit_log_event_type_check + CHECK (event_type IN ('query', 'role_change', 'grant_change', 'sso_config_change', 'secret_reveal', 'ai_interaction', 'agent_command')); diff --git a/proto/sentry/agent/v1/agent_control.pb.go b/proto/sentry/agent/v1/agent_control.pb.go index 3d42e3f..bfb69e5 100644 --- a/proto/sentry/agent/v1/agent_control.pb.go +++ b/proto/sentry/agent/v1/agent_control.pb.go @@ -21,6 +21,63 @@ const ( _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) ) +// AgentCommand is a one-shot action, not a persistent desired state like +// DesiredOverride -- delivered at-most-once (see CheckInResponse's +// comment). Scoped deliberately narrow: only RESTART exists today. +// STOP and UNINSTALL are real, disclosed future work, not oversights -- +// both need genuine OS service-manager integration (systemd's +// Restart=/RestartPreventExitStatus= semantics vs. Windows SCM recovery +// options are different enough per platform that hand-waving them would +// be dishonest), which RESTART doesn't: a graceful shutdown followed by +// a clean process exit, relying on whatever restart policy the host's +// service manager already has configured -- the same contract systemd/ +// SCM already expect from any well-behaved service. +type AgentCommand int32 + +const ( + AgentCommand_AGENT_COMMAND_UNSPECIFIED AgentCommand = 0 + AgentCommand_AGENT_COMMAND_RESTART AgentCommand = 1 +) + +// Enum value maps for AgentCommand. +var ( + AgentCommand_name = map[int32]string{ + 0: "AGENT_COMMAND_UNSPECIFIED", + 1: "AGENT_COMMAND_RESTART", + } + AgentCommand_value = map[string]int32{ + "AGENT_COMMAND_UNSPECIFIED": 0, + "AGENT_COMMAND_RESTART": 1, + } +) + +func (x AgentCommand) Enum() *AgentCommand { + p := new(AgentCommand) + *p = x + return p +} + +func (x AgentCommand) String() string { + return protoimpl.X.EnumStringOf(x.Descriptor(), protoreflect.EnumNumber(x)) +} + +func (AgentCommand) Descriptor() protoreflect.EnumDescriptor { + return file_sentry_agent_v1_agent_control_proto_enumTypes[0].Descriptor() +} + +func (AgentCommand) Type() protoreflect.EnumType { + return &file_sentry_agent_v1_agent_control_proto_enumTypes[0] +} + +func (x AgentCommand) Number() protoreflect.EnumNumber { + return protoreflect.EnumNumber(x) +} + +// Deprecated: Use AgentCommand.Descriptor instead. +func (AgentCommand) EnumDescriptor() ([]byte, []int) { + return file_sentry_agent_v1_agent_control_proto_rawDescGZIP(), []int{0} +} + // ReportedConfig is what an agent tells the platform about itself -- // read-only, for inventory/visibility. Deliberately excludes tls/ingest // endpoint fields: those are never reported and never remotely @@ -296,10 +353,24 @@ type CheckInResponse struct { state protoimpl.MessageState `protogen:"open.v1"` // False when no override has ever been set for this agent -- it // should be running whatever agent.toml already has, untouched. - HasOverride bool `protobuf:"varint,1,opt,name=has_override,json=hasOverride,proto3" json:"has_override,omitempty"` - Override *DesiredOverride `protobuf:"bytes,2,opt,name=override,proto3" json:"override,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + HasOverride bool `protobuf:"varint,1,opt,name=has_override,json=hasOverride,proto3" json:"has_override,omitempty"` + Override *DesiredOverride `protobuf:"bytes,2,opt,name=override,proto3" json:"override,omitempty"` + // AGENT_COMMAND_UNSPECIFIED when there's nothing to do. Unlike + // DesiredOverride, there is no "applied_command_version" echoed back + // in CheckInRequest: the server clears a pending command the moment + // it hands it out in a response (see + // ingest/internal/agentregistry.Registry.CheckIn), not once the agent + // confirms execution -- a restarting agent's process is gone before + // it could ever send that confirmation. This is an honest at-most- + // once delivery, not at-least-once: a command lost to a network + // failure between this response and the agent acting on it is simply + // lost, same as any fire-and-forget signal. Re-issuing (PUT + // /agents/{host}/command again) is the operator's recourse, same as + // it would be for a `systemctl restart` that silently failed to reach + // its target. + PendingCommand AgentCommand `protobuf:"varint,3,opt,name=pending_command,json=pendingCommand,proto3,enum=sentry.agent.v1.AgentCommand" json:"pending_command,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *CheckInResponse) Reset() { @@ -346,6 +417,13 @@ func (x *CheckInResponse) GetOverride() *DesiredOverride { return nil } +func (x *CheckInResponse) GetPendingCommand() AgentCommand { + if x != nil { + return x.PendingCommand + } + return AgentCommand_AGENT_COMMAND_UNSPECIFIED +} + var File_sentry_agent_v1_agent_control_proto protoreflect.FileDescriptor const file_sentry_agent_v1_agent_control_proto_rawDesc = "" + @@ -376,10 +454,14 @@ const file_sentry_agent_v1_agent_control_proto_rawDesc = "" + "\x18_batch_flush_interval_msB\x14\n" + "\x12_heartbeat_enabledB\x18\n" + "\x16_heartbeat_interval_msB\x10\n" + - "\x0e_journald_unit\"r\n" + + "\x0e_journald_unit\"\xba\x01\n" + "\x0fCheckInResponse\x12!\n" + "\fhas_override\x18\x01 \x01(\bR\vhasOverride\x12<\n" + - "\boverride\x18\x02 \x01(\v2 .sentry.agent.v1.DesiredOverrideR\boverride2\\\n" + + "\boverride\x18\x02 \x01(\v2 .sentry.agent.v1.DesiredOverrideR\boverride\x12F\n" + + "\x0fpending_command\x18\x03 \x01(\x0e2\x1d.sentry.agent.v1.AgentCommandR\x0ependingCommand*H\n" + + "\fAgentCommand\x12\x1d\n" + + "\x19AGENT_COMMAND_UNSPECIFIED\x10\x00\x12\x19\n" + + "\x15AGENT_COMMAND_RESTART\x10\x012\\\n" + "\fAgentControl\x12L\n" + "\aCheckIn\x12\x1f.sentry.agent.v1.CheckInRequest\x1a .sentry.agent.v1.CheckInResponseB8Z6github.com/sentry/sentry/proto/sentry/agent/v1;agentv1b\x06proto3" @@ -395,23 +477,26 @@ func file_sentry_agent_v1_agent_control_proto_rawDescGZIP() []byte { return file_sentry_agent_v1_agent_control_proto_rawDescData } +var file_sentry_agent_v1_agent_control_proto_enumTypes = make([]protoimpl.EnumInfo, 1) var file_sentry_agent_v1_agent_control_proto_msgTypes = make([]protoimpl.MessageInfo, 4) var file_sentry_agent_v1_agent_control_proto_goTypes = []any{ - (*ReportedConfig)(nil), // 0: sentry.agent.v1.ReportedConfig - (*CheckInRequest)(nil), // 1: sentry.agent.v1.CheckInRequest - (*DesiredOverride)(nil), // 2: sentry.agent.v1.DesiredOverride - (*CheckInResponse)(nil), // 3: sentry.agent.v1.CheckInResponse + (AgentCommand)(0), // 0: sentry.agent.v1.AgentCommand + (*ReportedConfig)(nil), // 1: sentry.agent.v1.ReportedConfig + (*CheckInRequest)(nil), // 2: sentry.agent.v1.CheckInRequest + (*DesiredOverride)(nil), // 3: sentry.agent.v1.DesiredOverride + (*CheckInResponse)(nil), // 4: sentry.agent.v1.CheckInResponse } var file_sentry_agent_v1_agent_control_proto_depIdxs = []int32{ - 0, // 0: sentry.agent.v1.CheckInRequest.current_config:type_name -> sentry.agent.v1.ReportedConfig - 2, // 1: sentry.agent.v1.CheckInResponse.override:type_name -> sentry.agent.v1.DesiredOverride - 1, // 2: sentry.agent.v1.AgentControl.CheckIn:input_type -> sentry.agent.v1.CheckInRequest - 3, // 3: sentry.agent.v1.AgentControl.CheckIn:output_type -> sentry.agent.v1.CheckInResponse - 3, // [3:4] is the sub-list for method output_type - 2, // [2:3] is the sub-list for method input_type - 2, // [2:2] is the sub-list for extension type_name - 2, // [2:2] is the sub-list for extension extendee - 0, // [0:2] is the sub-list for field type_name + 1, // 0: sentry.agent.v1.CheckInRequest.current_config:type_name -> sentry.agent.v1.ReportedConfig + 3, // 1: sentry.agent.v1.CheckInResponse.override:type_name -> sentry.agent.v1.DesiredOverride + 0, // 2: sentry.agent.v1.CheckInResponse.pending_command:type_name -> sentry.agent.v1.AgentCommand + 2, // 3: sentry.agent.v1.AgentControl.CheckIn:input_type -> sentry.agent.v1.CheckInRequest + 4, // 4: sentry.agent.v1.AgentControl.CheckIn:output_type -> sentry.agent.v1.CheckInResponse + 4, // [4:5] is the sub-list for method output_type + 3, // [3:4] is the sub-list for method input_type + 3, // [3:3] is the sub-list for extension type_name + 3, // [3:3] is the sub-list for extension extendee + 0, // [0:3] is the sub-list for field type_name } func init() { file_sentry_agent_v1_agent_control_proto_init() } @@ -425,13 +510,14 @@ func file_sentry_agent_v1_agent_control_proto_init() { File: protoimpl.DescBuilder{ GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: unsafe.Slice(unsafe.StringData(file_sentry_agent_v1_agent_control_proto_rawDesc), len(file_sentry_agent_v1_agent_control_proto_rawDesc)), - NumEnums: 0, + NumEnums: 1, NumMessages: 4, NumExtensions: 0, NumServices: 1, }, GoTypes: file_sentry_agent_v1_agent_control_proto_goTypes, DependencyIndexes: file_sentry_agent_v1_agent_control_proto_depIdxs, + EnumInfos: file_sentry_agent_v1_agent_control_proto_enumTypes, MessageInfos: file_sentry_agent_v1_agent_control_proto_msgTypes, }.Build() File_sentry_agent_v1_agent_control_proto = out.File diff --git a/proto/sentry/agent/v1/agent_control.proto b/proto/sentry/agent/v1/agent_control.proto index ed08dcd..14199d6 100644 --- a/proto/sentry/agent/v1/agent_control.proto +++ b/proto/sentry/agent/v1/agent_control.proto @@ -71,9 +71,39 @@ message DesiredOverride { string version = 6; } +// AgentCommand is a one-shot action, not a persistent desired state like +// DesiredOverride -- delivered at-most-once (see CheckInResponse's +// comment). Scoped deliberately narrow: only RESTART exists today. +// STOP and UNINSTALL are real, disclosed future work, not oversights -- +// both need genuine OS service-manager integration (systemd's +// Restart=/RestartPreventExitStatus= semantics vs. Windows SCM recovery +// options are different enough per platform that hand-waving them would +// be dishonest), which RESTART doesn't: a graceful shutdown followed by +// a clean process exit, relying on whatever restart policy the host's +// service manager already has configured -- the same contract systemd/ +// SCM already expect from any well-behaved service. +enum AgentCommand { + AGENT_COMMAND_UNSPECIFIED = 0; + AGENT_COMMAND_RESTART = 1; +} + message CheckInResponse { // False when no override has ever been set for this agent -- it // should be running whatever agent.toml already has, untouched. bool has_override = 1; DesiredOverride override = 2; + // AGENT_COMMAND_UNSPECIFIED when there's nothing to do. Unlike + // DesiredOverride, there is no "applied_command_version" echoed back + // in CheckInRequest: the server clears a pending command the moment + // it hands it out in a response (see + // ingest/internal/agentregistry.Registry.CheckIn), not once the agent + // confirms execution -- a restarting agent's process is gone before + // it could ever send that confirmation. This is an honest at-most- + // once delivery, not at-least-once: a command lost to a network + // failure between this response and the agent acting on it is simply + // lost, same as any fire-and-forget signal. Re-issuing (PUT + // /agents/{host}/command again) is the operator's recourse, same as + // it would be for a `systemctl restart` that silently failed to reach + // its target. + AgentCommand pending_command = 3; } diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 354fd44..1bd43d1 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -505,6 +505,15 @@ export type Agent = { applied_override_version: string; pending: boolean; updated_by?: string; + // pending_command/command_issued_at/by (real, restart only for now + // -- see /docs/agent-management-design.md's punch list) have no + // "delivered" signal to show the way config's `pending` does: + // ingest clears pending_command the instant it hands the command to + // the agent, not once the agent confirms it ran, so + // command_issued_at is shown as a last-issued record instead. + pending_command?: string; + command_issued_at?: string; + command_issued_by?: string; }; export function listAgents(): Promise { @@ -525,3 +534,14 @@ export function setAgentConfig(host: string, override: ConfigOverride): Promise< export function clearAgentConfig(host: string): Promise { return request(`/agents/${encodeURIComponent(host)}/config`, { method: 'DELETE' }); } + +// "restart" is the only supported value today -- server-validated +// (400 on anything else), RoleAdmin-gated (403 for a Viewer/Editor +// session), and audit-logged. See /docs/agent-management-design.md's +// punch list for why stop/uninstall aren't here yet. +export function issueAgentCommand(host: string, command: 'restart'): Promise { + return request(`/agents/${encodeURIComponent(host)}/command`, { + method: 'PUT', + body: JSON.stringify({ command }) + }); +} diff --git a/web/src/routes/agents/[host]/+page.svelte b/web/src/routes/agents/[host]/+page.svelte index 7c34f39..9eaafc7 100644 --- a/web/src/routes/agents/[host]/+page.svelte +++ b/web/src/routes/agents/[host]/+page.svelte @@ -1,6 +1,6 @@