From 87ee3cd449e47b8c9f2214c64f46e0ee92bc1ed8 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sun, 23 Aug 2026 23:39:20 +0100 Subject: [PATCH] Fix command policy after agent reinstall Project freshly minted installer command intent onto the stable host before returning remote config, and consume that intent once so later admin policy changes remain authoritative. --- docs/AGENT_SECURITY.md | 11 ++- .../v6/internal/subsystems/agent-lifecycle.md | 24 +++++ .../v6/internal/subsystems/api-contracts.md | 20 ++++ .../internal/subsystems/security-privacy.md | 10 ++ .../internal/subsystems/storage-recovery.md | 14 +++ frontend-modern/public/docs/AGENT_SECURITY.md | 11 ++- internal/api/agent_ingest.go | 93 +++++++++++++++++++ .../api/agent_install_command_shared_test.go | 33 +++++++ internal/api/agenttokens/install.go | 29 +++++- internal/api/agenttokens/install_test.go | 28 ++++++ ...config_handlers_cluster_additional_test.go | 8 +- .../api/configapi/config_setup_handlers.go | 10 +- internal/api/configapi/dependencies.go | 13 ++- internal/api/host_agent_install_token_test.go | 7 ++ ...gent_removal_lifecycle_integration_test.go | 93 +++++++++++++++++++ internal/api/security_regression_test.go | 29 ++++++ 16 files changed, 414 insertions(+), 19 deletions(-) diff --git a/docs/AGENT_SECURITY.md b/docs/AGENT_SECURITY.md index 593ee829ae..5dda2eb377 100644 --- a/docs/AGENT_SECURITY.md +++ b/docs/AGENT_SECURITY.md @@ -71,10 +71,13 @@ code: the service commonly runs as root, so only administrators should be able to replace them. Agent command tokens must be bound to a host or agent identity before command -registration is accepted. Proxmox install-command tokens are the only first-use -exception: because the server mints them before the installer knows the final -hostname, Pulse binds them to the first command agent that registers with that -token. Generic unbound `agent:exec` tokens still fail closed. +registration is accepted. Pulse-minted install-command tokens for the generic +host and Proxmox flows are the only first-use exception: because the server +mints them before the installer knows the final hostname, Pulse binds them to +the first command agent that registers with that token. Those tokens also carry +the operator's command-policy choice into the first accepted report so a stale +policy from an earlier installation cannot immediately disable the replacement +agent. Generic unbound `agent:exec` tokens still fail closed. ## Proxmox Deployment Choices diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index a8108b5d77..bbdcea84ed 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -2757,6 +2757,30 @@ for IP literals), so short-name vs fully-qualified drift does not break admission. Legacy pre-v6.1.1 hostname-bound records still migrate exactly once on hostname match. +### Install-token command intent converges once on the accepted host + +The generic host and PVE/PBS install-command endpoints stamp every newly +minted token with the operator's explicit enabled/disabled command-policy +intent. On the first report accepted for that token, +`reconcileInstallTokenCommandPolicy` re-reads the live token record, requires +the same `evaluateAgentExecBinding` identity decision used by the command +channel, projects enabled intent only while `agent:exec` is still present, and +persists the effective choice through `Monitor.UpdateHostAgentConfig` before +the report acknowledgement is built. This closes the #1728 reinstall failure: +a stable machine ID may retain an older `commandsEnabled=false` override, but +a fresh commands-enabled install no longer receives the stale override that +would stop its command client one second after startup. + +The token records the accepted host ID after the projection. That marker makes +install intent one-shot, so a later server-side command-policy change remains +authoritative instead of being undone on every report. Hand-created API tokens, +legacy records without explicit intent metadata, and tokens whose shared +binding decision rejects the reporting identity never enter this convergence +path. `TestHostAgentFreshInstallTokenReplacesStaleDisabledCommandPolicyOnce` +pins stale-policy repair and subsequent-admin-policy preservation; +`TestHandleAgentInstallCommand_HostWithCommands` and +`TestHandleAgentInstallCommand_HostWithoutCommands` pin the mint-time intent. + ### PBS connection composition does not create agent lifecycle evidence The shared API connections ledger and diagnostics now project PBS health from diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 20a80708a2..75a2853ec3 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -4229,6 +4229,26 @@ canonical hostname. A token-named resource never falls back through agent ID or hostname independently, so token rotation restores control for the same installed agent without permitting cross-host dispatch (#1728). +### Agent install command policy is a one-shot first-report contract + +`POST /api/agent-install-command` now binds its `enableCommands` request choice +to the minted token as server-authored `command_policy_intent` metadata for +both generic host and self-hosted PVE/PBS installers. Hosted tenant install +commands remain outside this convergence path. The token response shape is +unchanged and the metadata is not caller-controlled API input. + +When the token first reports an admitted host identity, the report handler +applies that intent to the host's durable desired configuration before building +the acknowledgement, then persists `command_policy_applied_agent_id`. Thus the +same response cannot carry an older disabled override after the installer +explicitly requested `--enable-commands`, while the marker prevents subsequent +reports from overwriting a later administrator policy decision. Enabled intent +without a live `agent:exec` scope becomes effective-disabled, and generic or +identity-rejected tokens remain outside the path. The report and config payload +schemas remain unchanged. The exact regression is pinned by +`TestHostAgentFreshInstallTokenReplacesStaleDisabledCommandPolicyOnce`; +minted enabled/disabled metadata is pinned by the agent install command tests. + ### Agent update target responses are reconciliation-safe `GET` and `HEAD /api/agent/version` project diff --git a/docs/release-control/v6/internal/subsystems/security-privacy.md b/docs/release-control/v6/internal/subsystems/security-privacy.md index 4e3d317f69..62b80036b8 100644 --- a/docs/release-control/v6/internal/subsystems/security-privacy.md +++ b/docs/release-control/v6/internal/subsystems/security-privacy.md @@ -1117,6 +1117,16 @@ binding version through the first-use path, an unrelated hostname still fails closed, and the registration-bound hostname is never overwritten by an equivalent spelling the agent reports, because the install grant compares against it. +Pulse-minted install tokens now carry a server-authored command-policy intent +in addition to first-use binding metadata. That intent grants no independent +authority: first-report convergence requires the same shared binding decision +as command-channel admission, and enabled intent is projected only while the +live token still has `agent:exec`. A generic API token cannot opt itself into +this path by holding `agent:exec`, and the applied-agent marker makes the +installer choice one-shot so it cannot override a later administrator disable. +This is the security side of #1728's stale-policy repair: reinstall intent may +replace policy retained from an earlier installation exactly once, without +turning reusable bearer credentials into command credentials. Telemetry/privacy disclosures now also route through the shipped frontend docs boundary: `frontend-modern/src/utils/docsLinks.ts` is the canonical frontend owner for privacy-document URLs, while `frontend-modern/public/docs/PRIVACY.md` diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 8f28424875..118bcee065 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -2242,6 +2242,20 @@ capability over storage or recovery data. ## Current State +### Fresh agent command policy preserves the existing recovery authority boundary + +New self-hosted install-command tokens project their explicit command-policy +intent onto the accepted host exactly once before the first report +acknowledgement. This repairs stale disabled policy after agent reinstall but +does not create a new storage recovery path: enabled intent still requires the +shared exec-token binding decision and live `agent:exec` scope, generic tokens +remain fail-closed, and every Docker update, host update, cleanup, or recovery +action continues through its existing late-bound readiness, approval, dispatch, +receipt, and verification authority. The one-shot applied-agent marker also +prevents later reports from undoing an administrator disable. The authenticated +reinstall regression is pinned by +`internal/api/host_agent_removal_lifecycle_integration_test.go`. + ### Proxmox backup views use canonical workflow routes The Proxmox backup surface exposes its two operator workflows as explicit diff --git a/frontend-modern/public/docs/AGENT_SECURITY.md b/frontend-modern/public/docs/AGENT_SECURITY.md index 593ee829ae..5dda2eb377 100644 --- a/frontend-modern/public/docs/AGENT_SECURITY.md +++ b/frontend-modern/public/docs/AGENT_SECURITY.md @@ -71,10 +71,13 @@ code: the service commonly runs as root, so only administrators should be able to replace them. Agent command tokens must be bound to a host or agent identity before command -registration is accepted. Proxmox install-command tokens are the only first-use -exception: because the server mints them before the installer knows the final -hostname, Pulse binds them to the first command agent that registers with that -token. Generic unbound `agent:exec` tokens still fail closed. +registration is accepted. Pulse-minted install-command tokens for the generic +host and Proxmox flows are the only first-use exception: because the server +mints them before the installer knows the final hostname, Pulse binds them to +the first command agent that registers with that token. Those tokens also carry +the operator's command-policy choice into the first accepted report so a stale +policy from an earlier installation cannot immediately disable the replacement +agent. Generic unbound `agent:exec` tokens still fail closed. ## Proxmox Deployment Choices diff --git a/internal/api/agent_ingest.go b/internal/api/agent_ingest.go index 59bd513730..7defdaf9dd 100644 --- a/internal/api/agent_ingest.go +++ b/internal/api/agent_ingest.go @@ -10,6 +10,7 @@ import ( "sync" "time" + "github.com/rcourtman/pulse-go-rewrite/internal/api/agenttokens" "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" @@ -101,6 +102,18 @@ func (h *UnifiedAgentHandlers) HandleReport(w http.ResponseWriter, r *http.Reque return } + // A freshly generated install token is the server's durable record of the + // operator's command-policy choice. Project that choice onto the stable host + // identity before returning remote config, otherwise an old false override + // survives remove/reinstall and immediately shuts down an agent launched + // with --enable-commands (#1728). + if err := reconcileInstallTokenCommandPolicy(h.getMonitor(r.Context()), tokenRecord, host); err != nil { + log.Error().Err(err). + Str("agent_id", host.ID). + Str("token_id", tokenID(tokenRecord)). + Msg("Failed to reconcile install-token command policy") + } + log.Debug(). Str("agentId", host.ID). Str("hostname", host.Hostname). @@ -137,6 +150,86 @@ func (h *UnifiedAgentHandlers) HandleReport(w http.ResponseWriter, r *http.Reque } } +func reconcileInstallTokenCommandPolicy(monitor *monitoring.Monitor, requestRecord *config.APITokenRecord, host models.Host) error { + if monitor == nil || requestRecord == nil || strings.TrimSpace(requestRecord.ID) == "" || strings.TrimSpace(host.ID) == "" { + return nil + } + + cfg := monitor.GetConfig() + if cfg == nil { + return nil + } + + // Authentication stores a clone in the request context. Re-read the live + // record so concurrent scope changes and an already-consumed intent marker + // are authoritative. + var record *config.APITokenRecord + config.Mu.RLock() + for index := range cfg.APITokens { + if cfg.APITokens[index].ID == requestRecord.ID { + clone := cfg.APITokens[index].Clone() + record = &clone + break + } + } + config.Mu.RUnlock() + if record == nil { + return nil + } + + requestedEnabled, hasIntent := agenttokens.ParseCommandPolicyIntent(record) + if !hasIntent || strings.TrimSpace(record.Metadata[agenttokens.CommandPolicyAppliedAgentIDMetadataKey]) != "" { + return nil + } + if !evaluateAgentExecBinding(record, host.ID, host.Hostname).admit { + // Intent metadata is useful only on a Pulse-minted install token whose + // binding policy admits this identity. Hand-created API tokens remain + // unable to turn themselves into command credentials. + return nil + } + + // Scope removal always wins over the original enabled intent. Conversely, + // adding agent:exec later does not rewrite a deliberately disabled install + // intent; the explicit token-scope editor and desired policy remain separate + // controls. + effectiveEnabled := requestedEnabled && record.HasScope(config.ScopeAgentExec) + if err := monitor.UpdateHostAgentConfig(host.ID, &effectiveEnabled); err != nil { + return fmt.Errorf("persist desired command policy: %w", err) + } + + persistence := monitor.GetConfigPersistence() + config.Mu.Lock() + defer config.Mu.Unlock() + for index := range cfg.APITokens { + live := &cfg.APITokens[index] + if live.ID != requestRecord.ID { + continue + } + if strings.TrimSpace(live.Metadata[agenttokens.CommandPolicyAppliedAgentIDMetadataKey]) != "" { + return nil + } + if live.Metadata == nil { + live.Metadata = make(map[string]string) + } + live.Metadata[agenttokens.CommandPolicyAppliedAgentIDMetadataKey] = host.ID + if persistence != nil { + if err := persistence.SaveAPITokens(cfg.APITokens); err != nil { + delete(live.Metadata, agenttokens.CommandPolicyAppliedAgentIDMetadataKey) + return fmt.Errorf("persist install-token command-policy marker: %w", err) + } + } + log.Info(). + Str("agent_id", host.ID). + Str("hostname", host.Hostname). + Str("token_id", live.ID). + Bool("commands_enabled", effectiveEnabled). + Msg("Applied install-token command policy to host") + return nil + } + + return nil +} + // HandleLookup returns agent registration details for installer validation. func (h *UnifiedAgentHandlers) HandleLookup(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { diff --git a/internal/api/agent_install_command_shared_test.go b/internal/api/agent_install_command_shared_test.go index 3809b33f84..5f480419d8 100644 --- a/internal/api/agent_install_command_shared_test.go +++ b/internal/api/agent_install_command_shared_test.go @@ -1,11 +1,15 @@ package api import ( + "bytes" + "encoding/json" + "net/http" "net/http/httptest" "strings" "testing" "time" + "github.com/rcourtman/pulse-go-rewrite/internal/api/agenttokens" "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/stretchr/testify/require" ) @@ -20,6 +24,35 @@ func TestNormalizeProxmoxInstallType(t *testing.T) { require.Equal(t, "Type must be 'pve' or 'pbs'", err.Error()) } +func TestAgentInstallCommandCarriesCommandPolicyIntent(t *testing.T) { + cfg := &config.Config{DataPath: t.TempDir(), AuthUser: "admin", AuthPass: "hashed-password"} + handler := newTestConfigHandlers(t, cfg) + req := httptest.NewRequest( + http.MethodPost, + "/api/agent-install-command", + bytes.NewBufferString(`{"type":"pve","enableCommands":true}`), + ) + req.Host = "pulse.example:7655" + rec := httptest.NewRecorder() + handler.HandleAgentInstallCommand(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("install command status = %d: %s", rec.Code, rec.Body.String()) + } + var response AgentInstallCommandResponse + if err := json.Unmarshal(rec.Body.Bytes(), &response); err != nil { + t.Fatalf("decode install command response: %v", err) + } + if len(cfg.APITokens) != 1 || response.Token == "" { + t.Fatalf("install command did not mint one token: response=%+v tokens=%d", response, len(cfg.APITokens)) + } + if got := cfg.APITokens[0].Metadata[agenttokens.CommandPolicyIntentMetadataKey]; got != agenttokens.CommandPolicyIntentEnabled { + t.Fatalf("command policy intent = %q, want enabled", got) + } + if !strings.Contains(response.Command, "--enable-commands") { + t.Fatalf("commands-enabled install omitted flag: %s", response.Command) + } +} + func TestBuildProxmoxAgentInstallCommand(t *testing.T) { command := buildProxmoxAgentInstallCommand(agentInstallCommandOptions{ BaseURL: "https://pulse.example.com/", diff --git a/internal/api/agenttokens/install.go b/internal/api/agenttokens/install.go index 4d58bb1ae5..bb58b68c7a 100644 --- a/internal/api/agenttokens/install.go +++ b/internal/api/agenttokens/install.go @@ -12,8 +12,12 @@ import ( ) const ( - IssuedAtMetadataKey = "install_issued_at" - OwnerUserIDMetadataKey = "owner_user_id" + IssuedAtMetadataKey = "install_issued_at" + OwnerUserIDMetadataKey = "owner_user_id" + CommandPolicyIntentMetadataKey = "command_policy_intent" + CommandPolicyAppliedAgentIDMetadataKey = "command_policy_applied_agent_id" + CommandPolicyIntentEnabled = "enabled" + CommandPolicyIntentDisabled = "disabled" ) var ( @@ -53,6 +57,27 @@ func HostScopes(enableCommands bool) []string { return scopes } +func CommandPolicyIntent(enableCommands bool) string { + if enableCommands { + return CommandPolicyIntentEnabled + } + return CommandPolicyIntentDisabled +} + +func ParseCommandPolicyIntent(record *config.APITokenRecord) (bool, bool) { + if record == nil { + return false, false + } + switch strings.TrimSpace(record.Metadata[CommandPolicyIntentMetadataKey]) { + case CommandPolicyIntentEnabled: + return true, true + case CommandPolicyIntentDisabled: + return false, true + default: + return false, false + } +} + func IssueAndPersist(cfg *config.Config, persistence *config.ConfigPersistence, opts IssueOptions) (string, *config.APITokenRecord, error) { if cfg == nil { return "", nil, fmt.Errorf("config is required") diff --git a/internal/api/agenttokens/install_test.go b/internal/api/agenttokens/install_test.go index 127cadb184..9f6c9742a3 100644 --- a/internal/api/agenttokens/install_test.go +++ b/internal/api/agenttokens/install_test.go @@ -38,3 +38,31 @@ func TestIssueAndPersistRejectsReservedOwnerMetadata(t *testing.T) { t.Fatalf("reserved owner metadata error = %v, want ErrRecord", err) } } + +func TestCommandPolicyIntentRoundTrip(t *testing.T) { + for _, tc := range []struct { + name string + value string + enabled bool + valid bool + }{ + {name: "enabled", value: CommandPolicyIntent(true), enabled: true, valid: true}, + {name: "disabled", value: CommandPolicyIntent(false), enabled: false, valid: true}, + {name: "missing", value: "", enabled: false, valid: false}, + {name: "invalid", value: "yes", enabled: false, valid: false}, + } { + t.Run(tc.name, func(t *testing.T) { + record := &config.APITokenRecord{Metadata: map[string]string{ + CommandPolicyIntentMetadataKey: tc.value, + }} + enabled, valid := ParseCommandPolicyIntent(record) + if enabled != tc.enabled || valid != tc.valid { + t.Fatalf("ParseCommandPolicyIntent() = (%v, %v), want (%v, %v)", enabled, valid, tc.enabled, tc.valid) + } + }) + } + + if _, valid := ParseCommandPolicyIntent(nil); valid { + t.Fatal("nil record unexpectedly carried command-policy intent") + } +} diff --git a/internal/api/configapi/config_handlers_cluster_additional_test.go b/internal/api/configapi/config_handlers_cluster_additional_test.go index 94b9e8ae7d..3bf2ab49b1 100644 --- a/internal/api/configapi/config_handlers_cluster_additional_test.go +++ b/internal/api/configapi/config_handlers_cluster_additional_test.go @@ -525,7 +525,7 @@ func TestHandleAgentInstallCommand(t *testing.T) { cfg := &config.Config{DataPath: t.TempDir(), AuthUser: "admin", AuthPass: "hashed-password"} handler := newTestConfigHandlers(t, cfg) - body := []byte(`{"type":"pve"}`) + body := []byte(`{"type":"pve","enableCommands":true}`) req := httptest.NewRequest(http.MethodPost, "/api/config/agent-install", bytes.NewReader(body)) req.Host = "example.com:8080" rec := httptest.NewRecorder() @@ -551,6 +551,12 @@ func TestHandleAgentInstallCommand(t *testing.T) { if len(cfg.APITokens) != 1 { t.Fatalf("expected API token to be persisted") } + if got := cfg.APITokens[0].Metadata[agentInstallCommandPolicyIntentKey]; got != "enabled" { + t.Fatalf("command_policy_intent metadata = %q, want enabled", got) + } + if !bytes.Contains([]byte(resp.Command), []byte("--enable-commands")) { + t.Fatalf("expected command execution flag in command: %s", resp.Command) + } } func TestHandleAgentInstallCommand_InsecureHTTPS(t *testing.T) { diff --git a/internal/api/configapi/config_setup_handlers.go b/internal/api/configapi/config_setup_handlers.go index 5e63b87275..fa4d4c3703 100644 --- a/internal/api/configapi/config_setup_handlers.go +++ b/internal/api/configapi/config_setup_handlers.go @@ -2571,8 +2571,9 @@ func (h *ConfigHandlers) handleAgentInstallCommand(w http.ResponseWriter, r *htt TokenName: tokenName, OwnerUserID: h.apiTokenOwnerUserIDForRequest(cfg, r), Metadata: map[string]string{ - "install_type": installType, - "issued_via": "config_agent_install_command", + "install_type": installType, + "issued_via": "config_agent_install_command", + agentInstallCommandPolicyIntentKey: agentInstallCommandPolicyIntent(req.EnableCommands), }, }) if err != nil { @@ -2643,8 +2644,9 @@ func (h *ConfigHandlers) handleHostAgentInstallToken(w http.ResponseWriter, r *h OwnerUserID: h.apiTokenOwnerUserIDForRequest(cfg, r), Scopes: hostAgentInstallScopes(req.EnableCommands), Metadata: map[string]string{ - "install_type": agentInstallTypeHost, - "issued_via": agentInstallIssuedViaConfig, + "install_type": agentInstallTypeHost, + "issued_via": agentInstallIssuedViaConfig, + agentInstallCommandPolicyIntentKey: agentInstallCommandPolicyIntent(req.EnableCommands), }, }) if err != nil { diff --git a/internal/api/configapi/dependencies.go b/internal/api/configapi/dependencies.go index 81bb1a95a9..c5532123e8 100644 --- a/internal/api/configapi/dependencies.go +++ b/internal/api/configapi/dependencies.go @@ -15,10 +15,11 @@ import ( ) const ( - OrgIDContextKey = apicontext.OrgIDContextKey - agentInstallIssuedViaConfig = "config_agent_install_command" - agentInstallIssuedViaHosted = "hosted_agent_install_command" - agentInstallTypeHost = "host" + OrgIDContextKey = apicontext.OrgIDContextKey + agentInstallIssuedViaConfig = "config_agent_install_command" + agentInstallIssuedViaHosted = "hosted_agent_install_command" + agentInstallTypeHost = "host" + agentInstallCommandPolicyIntentKey = agenttokens.CommandPolicyIntentMetadataKey ) var ( @@ -120,6 +121,10 @@ func hostAgentInstallScopes(enableCommands bool) []string { return agenttokens.HostScopes(enableCommands) } +func agentInstallCommandPolicyIntent(enableCommands bool) string { + return agenttokens.CommandPolicyIntent(enableCommands) +} + func (h *ConfigHandlers) authConfiguredForAgentLifecycle(cfg *config.Config) bool { return h.runtimeDependencies().AuthConfigured(cfg) } diff --git a/internal/api/host_agent_install_token_test.go b/internal/api/host_agent_install_token_test.go index e4c0f6ba7d..a137eb9b3d 100644 --- a/internal/api/host_agent_install_token_test.go +++ b/internal/api/host_agent_install_token_test.go @@ -7,6 +7,7 @@ import ( "net/http/httptest" "testing" + "github.com/rcourtman/pulse-go-rewrite/internal/api/agenttokens" "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" ) @@ -58,6 +59,9 @@ func TestHandleAgentInstallCommand_HostWithCommands(t *testing.T) { if got := record.Metadata["issued_via"]; got != agentInstallIssuedViaConfig { t.Fatalf("issued_via metadata = %q, want %q", got, agentInstallIssuedViaConfig) } + if got := record.Metadata[agenttokens.CommandPolicyIntentMetadataKey]; got != "enabled" { + t.Fatalf("command_policy_intent metadata = %q, want enabled", got) + } if !canBindAgentInstallExecToken(&record, "agent-nuc", "nuc") { t.Fatalf("expected host install token to be eligible for first-use exec binding") } @@ -83,6 +87,9 @@ func TestHandleAgentInstallCommand_HostWithoutCommands(t *testing.T) { if record.HasScope(config.ScopeAgentExec) { t.Fatalf("expected no exec scope without enableCommands, got %v", record.Scopes) } + if got := record.Metadata[agenttokens.CommandPolicyIntentMetadataKey]; got != "disabled" { + t.Fatalf("command_policy_intent metadata = %q, want disabled", got) + } } func TestHandleAgentInstallCommand_HostOmitsTokenWhenAuthOptional(t *testing.T) { diff --git a/internal/api/host_agent_removal_lifecycle_integration_test.go b/internal/api/host_agent_removal_lifecycle_integration_test.go index f0b07d764a..079df408dd 100644 --- a/internal/api/host_agent_removal_lifecycle_integration_test.go +++ b/internal/api/host_agent_removal_lifecycle_integration_test.go @@ -6,9 +6,11 @@ import ( "net/http" "net/http/httptest" "path/filepath" + "strings" "testing" "time" + "github.com/rcourtman/pulse-go-rewrite/internal/api/agenttokens" "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" @@ -332,6 +334,97 @@ func TestHostAgentRemovalLifecycleThroughAuthenticatedRouterAndRestart(t *testin } } +func TestHostAgentFreshInstallTokenReplacesStaleDisabledCommandPolicyOnce(t *testing.T) { + dataPath := t.TempDir() + const adminRaw = "issue-1728-admin-token-123.12345678" + admin := newTokenRecord(t, adminRaw, []string{config.ScopeWildcard}, nil) + if err := config.NewConfigPersistence(dataPath).SaveAPITokens([]config.APITokenRecord{admin}); err != nil { + t.Fatalf("SaveAPITokens: %v", err) + } + + runtime := newHostRemovalLifecycleHTTPRuntime(t, dataPath, []config.APITokenRecord{admin}) + t.Cleanup(runtime.stop) + + const hostID = "machine-stale-command-policy" + disabled := false + if err := runtime.monitor.UpdateHostAgentConfig(hostID, &disabled); err != nil { + t.Fatalf("seed stale command policy: %v", err) + } + + installRec := serveHostRemovalLifecycleRequest( + t, + runtime, + http.MethodPost, + "/api/agent-install-command", + adminRaw, + []byte(`{"type":"host","enableCommands":true,"name":"issue-1728-reinstall"}`), + ) + install := decodeHostAgentInstallResponse(t, installRec) + if install.Token == "" || install.Record == nil { + t.Fatalf("commands-enabled reinstall omitted token record: %+v", install) + } + + report := agentshost.Report{ + Agent: agentshost.AgentInfo{ + ID: "agent-stale-command-policy", + Version: "6.3.1", + Type: "unified", + CommandsEnabled: true, + }, + Host: agentshost.HostInfo{ + ID: hostID, + MachineID: hostID, + Hostname: "alpine-docker", + Platform: "linux", + }, + Timestamp: time.Now().UTC(), + } + + status, gotID, body := postHostRemovalLifecycleReport(t, runtime, install.Token, report) + if status != http.StatusOK || gotID != hostID { + t.Fatalf("fresh reinstall report = (%d, %q), want (200, %q): %s", status, gotID, hostID, body) + } + if !strings.Contains(body, `"commandsEnabled":true`) { + t.Fatalf("fresh reinstall acknowledgement retained stale disabled policy: %s", body) + } + if desired := runtime.monitor.GetHostAgentConfig(hostID).CommandsEnabled; desired == nil || !*desired { + t.Fatalf("stored desired command policy = %#v, want true", desired) + } + + record, ok := runtime.config.ValidateAPIToken(install.Token) + if !ok || record.Metadata[agenttokens.CommandPolicyAppliedAgentIDMetadataKey] != hostID { + t.Fatalf("install-token applied marker = %#v, want agent %q", record, hostID) + } + persistedTokens, err := config.NewConfigPersistence(dataPath).LoadAPITokens() + if err != nil { + t.Fatalf("reload install token marker: %v", err) + } + persistedRecord := tokenRecordByID(persistedTokens, record.ID) + if persistedRecord == nil || persistedRecord.Metadata[agenttokens.CommandPolicyAppliedAgentIDMetadataKey] != hostID { + t.Fatalf("persisted install token marker = %#v, want agent %q", persistedRecord, hostID) + } + + // The install intent is one-shot. A later explicit server-side disable + // remains authoritative instead of being undone on every report. + if err := runtime.monitor.UpdateHostAgentConfig(hostID, &disabled); err != nil { + t.Fatalf("disable command policy after install convergence: %v", err) + } + report.Timestamp = report.Timestamp.Add(time.Minute) + status, _, body = postHostRemovalLifecycleReport(t, runtime, install.Token, report) + if status != http.StatusOK || !strings.Contains(body, `"commandsEnabled":false`) { + t.Fatalf("consumed install intent overrode later disabled policy: status=%d body=%s", status, body) + } +} + +func tokenRecordByID(records []config.APITokenRecord, tokenID string) *config.APITokenRecord { + for index := range records { + if records[index].ID == tokenID { + return &records[index] + } + } + return nil +} + // TestHostAgentRenameKeepsCommandGateAlignedWithChannelAdmission covers the // host-rename leg of the agent lifecycle: a host that renames itself (or // starts reporting a fully-qualified hostname) after its exec token was bound diff --git a/internal/api/security_regression_test.go b/internal/api/security_regression_test.go index 4376f226bd..cbe1ec0431 100644 --- a/internal/api/security_regression_test.go +++ b/internal/api/security_regression_test.go @@ -18,6 +18,7 @@ import ( "github.com/gorilla/websocket" "github.com/rcourtman/pulse-go-rewrite/internal/agentexec" "github.com/rcourtman/pulse-go-rewrite/internal/ai/approval" + "github.com/rcourtman/pulse-go-rewrite/internal/api/agenttokens" "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" @@ -31,6 +32,34 @@ type wsRawMessage struct { Payload json.RawMessage `json:"payload,omitempty"` } +func TestSecurityGenericExecTokenCannotApplyInstallCommandPolicy(t *testing.T) { + token := config.APITokenRecord{ + ID: "generic-exec-token", + Scopes: []string{config.ScopeAgentReport, config.ScopeAgentExec}, + } + cfg := &config.Config{DataPath: t.TempDir(), APITokens: []config.APITokenRecord{token}} + _, monitor := newUnifiedAgentHandlers(t, cfg) + + disabled := false + if err := monitor.UpdateHostAgentConfig("machine-1", &disabled); err != nil { + t.Fatalf("seed disabled command policy: %v", err) + } + if err := reconcileInstallTokenCommandPolicy(monitor, &token, models.Host{ + ID: "machine-1", + Hostname: "docker-1", + }); err != nil { + t.Fatalf("reconcile generic exec token: %v", err) + } + + desired := monitor.GetHostAgentConfig("machine-1").CommandsEnabled + if desired == nil || *desired { + t.Fatalf("generic exec token changed desired command policy: %#v", desired) + } + if got := cfg.APITokens[0].Metadata[agenttokens.CommandPolicyAppliedAgentIDMetadataKey]; got != "" { + t.Fatalf("generic exec token gained install-policy marker %q", got) + } +} + func TestDeprovisionSessionRevocationIncludesPersistedUntrackedSessions(t *testing.T) { resetSessionTracking() resetSessionStoreForTests()