Add option to enable graceful unenroll to invalid API key agents.

[blakerouse]

What is the problem this PR solves?

Once an Elastic Agent is force unenrolled the Elastic Agents will continue to communicate to Fleet Server. There are cases where it would be best to just have the Elastic Agent stop all of its components, unenroll if it can (those with tamper protection on will not be able to), and those that cannot will continue to receive 401's for a full hour, until the cycle starts again.

How does this PR solve the problem?

This changes the behavior of invalid API keys from being a 401 error that just gets retried non-stop to a 200 with a policy change action that is an empty policy, then a unenroll action, and then back to the 401 error.

1. The empty policy will cause the Elastic Agent to stop all of its running components.
2. The unenroll action will cause the Elastic Agent to stop communicating with Fleet (as long as there is not tamper protection enabled, if enabled this will not stop the Elastic Agent).
3. 401 original error returns for a full hour.

How to test this PR locally

1. Enroll an Elastic Agent.
2. Perform force unenroll in Fleet.
3. Wait for next check-in and see that it stops all of its running components.
4. Wait for follow-up check-in and see that it unenrolls.

Design Checklist

• I have ensured my design is stateless and will work when multiple fleet-server instances are behind a load balancer.
• [ ] I have or intend to scale test my changes, ensuring it will work reliably with 100K+ agents connected.
• [ ] I have included fail safe mechanisms to limit the load on fleet-server: rate limiting, circuit breakers, caching, load shedding, etc.

Checklist

• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have made corresponding change to the default configuration files
• I have added tests that prove my fix is effective or that my feature works
• I have added an entry in ./changelog/fragments using the changelog tool

[mergify]

This pull request does not have a backport label. Could you fix it @blakerouse? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-./d./d is the label to automatically backport to the 8./d branch. /d is the digit
• backport-active-all is the label that automatically backports to all active branches.
• backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
• backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

[Copilot]

Pull request overview

Adds a feature-flagged behavior in Fleet Server check-in authentication to return a 200 OK with a single UNENROLL action (instead of 401) when an agent checks in with an invalid/disabled API key, enabling agents to gracefully stop retrying after force-unenroll.

Changes:

• Introduces unenroll_on_invalid_api_key under inputs[].server.feature_flags and documents it in the reference config + changelog.
• Updates check-in handling to emit an UNENROLL action response when the flag is enabled and auth fails due to invalid/disabled API keys (or inactive agent).
• Adds integration + e2e coverage to validate the new behavior end-to-end (including real elastic-agent behavior).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
testing/e2e/testdata/stand-alone-https-unenroll.tpl	New standalone HTTPS config template enabling the feature flag for e2e.
testing/e2e/stand_alone_test.go	New e2e test validating elastic-agent self-unenroll behavior after API key invalidation.
internal/pkg/server/fleet_integration_test.go	New integration test validating 200+UNENROLL vs 401 behavior behind the flag.
internal/pkg/config/input.go	Adds the UnenrollOnInvalidAPIKey feature flag to config.
internal/pkg/api/handleCheckin.go	Implements UNENROLL response path for invalid/disabled API key auth errors.
internal/pkg/api/handleCheckin_test.go	Adds unit tests for invalid-key detection + UNENROLL response generation.
fleet-server.reference.yml	Documents the new unenroll_on_invalid_api_key setting and default.
changelog/fragments/1786137338-unenroll-on-invalid-api-key.yaml	Changelog entry describing the enhancement and configuration.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (4)

internal/pkg/api/handleCheckin_test.go:1145

• TestWriteUnenrollResponse calls writeUnenrollResponse with an extra *http.Request argument, but the method signature is writeUnenrollResponse(logger, w, agentID). This won’t compile as written.

	wr := httptest.NewRecorder()
	logger := testlog.SetLogger(t)

	err = ct.writeUnenrollResponse(logger, wr, agentID)
	require.NoError(t, err)

internal/pkg/api/handleCheckin.go:217

• This log message says “invalid API key”, but the UNENROLL response path is also used for inactive agent records (ErrAgentInactive). The message should reflect both cases to avoid misleading operational logs.

	zlog.Info().
		Str(ecs.AgentID, agentID).
		Str(ecs.ActionID, action.Id).
		Msg("Returning UNENROLL action for agent with invalid API key")

testing/e2e/stand_alone_test.go:746

• The doc comment says the agent “stops running”, but later in the test it notes that an unenrolled agent keeps running and stops checking in. This is inconsistent and can confuse future maintainers reading the test.

// The test observes only the elastic-agent's own log output — not fleet-server's API response —
// to confirm the agent processes the UNENROLL action and stops running.

internal/pkg/api/handleCheckin.go:200

• writeUnenrollResponse can be triggered for ErrAgentInactive (inactive agent record) as well as invalid/disabled API keys, but the comment currently states it is only used when the API key is invalid.

This issue also appears on line 214 of the same file.

// writeUnenrollResponse writes a 200 check-in response containing a single UNENROLL action.
// It is used when UnenrollOnInvalidAPIKey is enabled and the agent's API key is invalid.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (4)

internal/pkg/api/handleCheckin.go:201

• This comment states the UNENROLL response is used only for “invalid API key”, but this function is also used when the agent is inactive (ErrAgentInactive). Updating the comment avoids misleading documentation.

// writeUnenrollResponse writes a 200 check-in response containing a single UNENROLL action.
// It is used when UnenrollOnInvalidAPIKey is enabled and the agent's API key is invalid.
func (ct *CheckinT) writeUnenrollResponse(zlog zerolog.Logger, w http.ResponseWriter, agentID string) error {

internal/pkg/api/handleCheckin.go:192

• The comment says this helper detects “invalid or disabled API key” errors, but the implementation also treats ErrAgentInactive as a match. Please update the comment to reflect the actual behavior so future readers don’t miss that inactive-agent check-ins are also converted to UNENROLL when the flag is enabled.

This issue also appears on line 199 of the same file.

// isInvalidAPIKeyErr reports whether err represents an invalid or disabled API key
// that would normally produce a 401 response on check-in.
func isInvalidAPIKeyErr(err error) bool {

internal/pkg/api/handleCheckin.go:217

• The log message claims the API key is invalid, but this path can also be hit for inactive agents (ErrAgentInactive). Consider making the message more general to avoid incorrect operational signals.

		Msg("Returning UNENROLL action for agent with invalid API key")

testing/e2e/stand_alone_test.go:746

• This test comment says the agent “stops running”, but the assertions below verify it “stops checking in” while the process may continue running. Updating the wording will keep the test description consistent with the behavior being asserted.

// to confirm the agent processes the UNENROLL action and stops running.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (4)

internal/pkg/api/handleCheckin.go:347

• writeUnenrollResponse writes directly to the ResponseWriter without updating the check-in route body_out metric (cntCheckin.bodyOut), which will underreport egress for this new 200-response path when the feature flag is enabled.

	payload, err := json.Marshal(&resp)
	if err != nil {
		return fmt.Errorf("writeUnenrollResponse marshal: %w", err)
	}

	_, err = w.Write(payload)
	return err

internal/pkg/api/handleCheckin.go:314

• writeEmptyPolicyChangeResponse writes directly to the ResponseWriter without updating the check-in route body_out metric (cntCheckin.bodyOut), which will underreport egress for this new 200-response path when the feature flag is enabled.

This issue also appears on line 341 of the same file.

	payload, err := json.Marshal(&resp)
	if err != nil {
		return fmt.Errorf("writeEmptyPolicyChangeResponse marshal: %w", err)
	}

	_, err = w.Write(payload)
	return err

internal/pkg/api/handleCheckin.go:1513

• newInvalidKeyLRU can be configured with max_bytes smaller than a single entry (invalidKeyLRUEntryBytes), but Store() will still insert one element (c.l.Len()==0 prevents eviction), allowing used to exceed maxBytes. Clamping the configured cap to at least one entry size avoids violating the stated memory cap semantics for small values.

func newInvalidKeyLRU(maxBytes int64) *invalidKeyLRU {
	if maxBytes <= 0 {
		maxBytes = config.DefaultGracefulForceUnenrollMaxBytes
	}

internal/pkg/server/fleet.go:549

• The invalid API key state cleaner goroutine is started unconditionally. When graceful_force_unenroll is disabled this ticker loop does work (and holds an extra goroutine) without any possibility of state being present. Starting it only when the feature is enabled avoids unnecessary background load.

	ct, err := api.NewCheckinT(f.verCon, &cfg.Inputs[0].Server, f.cache, bc, pm, am, ad, bulker,
		api.WithOutputSecretCandidateCollector(outputSecretReconciler))
	if err != nil {
		return err
	}
	f.checkinT = ct
	g.Go(loggedRunFunc(ctx, "Invalid API key state cleaner", ct.RunInvalidKeyStateCleaner))
	et, err := api.NewEnrollerT(f.verCon, &cfg.Inputs[0].Server, bulker, f.cache)

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (2)

internal/pkg/api/handleCheckin.go:347

• writeUnenrollResponse writes the payload without recording the number of bytes written in cntCheckin.bodyOut (unlike writeResponse) and returns the raw write error without context. This makes check-in bodyOut metrics undercount when graceful_force_unenroll returns UNENROLL as a 200 response, and makes write failures harder to diagnose.

	_, err = w.Write(payload)
	return err

internal/pkg/api/handleCheckin.go:314

• writeEmptyPolicyChangeResponse writes the payload without recording the number of bytes written in cntCheckin.bodyOut (unlike writeResponse) and returns the raw write error without context. This makes check-in bodyOut metrics undercount when graceful_force_unenroll returns a 200 response, and makes write failures harder to diagnose.

This issue also appears on line 346 of the same file.

	_, err = w.Write(payload)
	return err

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (3)

internal/pkg/api/handleCheckin.go:275

• invalidKeyStates is updated via a Load → modify → Store sequence across multiple lock acquisitions. Concurrent invalid check-ins for the same agent can lose increments (e.g., two goroutines both observe count=0 and both store count=1), causing the escalation steps to be skipped/delayed unpredictably.

	var s invalidKeyState
	if loaded, ok := ct.invalidKeyStates.Load(agentID); ok {
		s = loaded
		if now.Sub(s.firstSeen) >= invalidKeyStateReset {
			ct.invalidKeyStates.Delete(agentID)

testing/e2e/stand_alone_test.go:745

• The test comments are internally inconsistent about UNENROLL behavior: here it says "disenrolls and exits", but later (lines 913-916) it says the process stays running and merely stops sending check-ins. This makes the test intent unclear and could mislead future debugging.

//  1. 1st invalid checkin → fleet-server returns POLICY_CHANGE with empty policy.
//     The agent stops all running components.
//  2. 2nd invalid checkin → fleet-server returns UNENROLL.
//     The agent disenrolls and exits.
//

internal/pkg/server/fleet.go:549

• RunInvalidKeyStateCleaner is started unconditionally. When graceful_force_unenroll is disabled (the default), this still spawns an extra goroutine + ticker even though the invalid-key LRU will never be populated.

	f.checkinT = ct
	g.Go(loggedRunFunc(ctx, "Invalid API key state cleaner", ct.RunInvalidKeyStateCleaner))

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (4)

internal/pkg/api/handleCheckin.go:251

• handleInvalidAPIKey stores escalation state keyed by the agentID path parameter even when authentication fails. Because agentID is attacker-controlled in that case, a client can use very large/non-UUID IDs to amplify memory usage and also bypass the LRU byte cap (which assumes a fixed per-entry size). Consider only tracking state for well-formed agent IDs (e.g., UUID) and otherwise pass through the original 401.

func (ct *CheckinT) handleInvalidAPIKey(zlog zerolog.Logger, w http.ResponseWriter, agentID string, origErr error) error {
	now := time.Now()

internal/pkg/api/handleCheckin.go:347

• writeUnenrollResponse also bypasses cntCheckin.bodyOut accounting, which can skew check-in egress metrics when this feature is enabled.

	_, err = w.Write(payload)
	return err

internal/pkg/api/handleCheckin.go:1519

• invalidKeyLRU can exceed its configured maxBytes when max_bytes is set smaller than a single entry (250 bytes): with an empty LRU the eviction loop won’t run and Store will still insert, making used > maxBytes. Clamping to a minimum of one entry avoids silently ignoring the cap.

func newInvalidKeyLRU(maxBytes int64) *invalidKeyLRU {
	if maxBytes <= 0 {
		maxBytes = config.DefaultGracefulForceUnenrollMaxBytes
	}

internal/pkg/api/handleCheckin.go:314

• These special-case check-in responses bypass cntCheckin.bodyOut accounting, so /stats will under-report check-in egress when graceful_force_unenroll is active. Capturing bytes written keeps metrics consistent with the normal writeResponse path.

This issue also appears on line 346 of the same file.

	_, err = w.Write(payload)
	return err

[github-actions]

TL;DR

Buildkite 16311 failed in Run check-ci because internal/pkg/api/handleCheckin_test.go calls processPolicy with the old function signature. Add the new policy.OutputSecretCandidateCollector argument to that test call to fix the compile break.

Remediation

• Update internal/pkg/api/handleCheckin_test.go at the failing call site to pass the new 6th arg to processPolicy (for this test, nil is the minimal valid value unless you want collector behavior asserted).
• Re-run mage check:fix (or .buildkite/scripts/check_ci.sh) to confirm go fix compiles across tags.

Investigation details

Root Cause

processPolicy now requires a 6th parameter: policy.OutputSecretCandidateCollector:

• internal/pkg/api/handleCheckin.go:897

But the test still calls the 5-arg form:

• internal/pkg/api/handleCheckin_test.go:1607

That signature mismatch causes compile failure during go fix in CI.

Evidence

• Build: https://buildkite.com/elastic/fleet-server/builds/16311
• Job/step: Run check-ci (.buildkite/scripts/check_ci.sh)
• Key log excerpt:

fix: internal/pkg/api/handleCheckin_test.go:1847:69: not enough arguments in call to processPolicy
  have (context.Context, zerolog.Logger, *testing.MockBulk, *model.Agent, *policy.ParsedPolicy)
  want (context.Context, zerolog.Logger, bulk.Bulk, *model.Agent, *policy.ParsedPolicy, policy.OutputSecretCandidateCollector)
Error: running "go fix ./..." failed with exit code 1

Verification

• Not run locally in this workflow; conclusion is based on direct compiler diagnostics in the Buildkite log and source signature/call-site inspection.

Follow-up

• This is a code-level compile mismatch (not infra/flaky). No retry-only mitigation needed.

---

What is this? | From workflow: PR Buildkite Detective

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

[swiatekm]

I have two questions:

1. If we tell the agent to unenroll on a 401, and the root cause in ES is fixable (by revalidating the key, or maybe the ES failure is itself infrastructural and temporary), then we're forcing the user to manually re-enroll with a new key later, right? At the very least, there should be a delay on this, I think.

2. Is there a reason we're not actually persisting the state to Fleet rather than storing it in the cache. Sounds like it'd be both simpler and more robust, even if it might require new fields in the index.

[cmacknz]

If we tell the agent to unenroll on a 401, and the root cause in ES is fixable (by revalidating the key, or maybe the ES failure is itself infrastructural and temporary), then we're forcing the user to manually re-enroll with a new key later, right? At the very least, there should be a delay on this, I think.

I am personally a bit hesitant to include UNENROLL in this for this reason, any kind of automatic unenroll we've ever introduced has lead to an incident of this sort. That said in this situation, the agents are already likely unmanaged and a users has opted into this, so it isn't automatic. If this weren't behind a feature flag it would be much more dangerous, though at the same time we could have people forget to turn it off and then get hit by this. We will eventually make unenroll continue to contact Fleet Server as part of the uninstall action anyway so maybe the unenroll has less value considering this, though until we do this work there is no other way to get agents to stop contacting fleet server.

Is there a reason we're not actually persisting the state to Fleet rather than storing it in the cache. Sounds like it'd be both simpler and more robust, even if it might require new fields in the index.

It leaves a permanent record of this feature in Elasticsearch, there is a chance we can remove this feature, or at least need for it should reduce greatly once we rework how unenroll works as part of the uninstall work. I'm also in favor of trying to keep this simple because there is some urgency to produce it for the affected user.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (3)

internal/pkg/api/handleCheckin.go:275

• handleInvalidAPIKey updates and stores the per-agent escalation state before confirming the step-1/step-2 response was successfully written. If writeEmptyPolicyChangeResponse/writeUnenrollResponse fails (e.g., client disconnect, marshal/write error), the state still advances, so the next retry may incorrectly skip to UNENROLL or pass-through 401 even though the agent never received the prior action. Store the new state only after a successful write (and consider capping the stored count at 3 since higher values don’t change behavior).

	s.count++
	if s.count == 1 {
		s.firstSeen = now
	}
	ct.invalidKeyStates.Store(agentID, s)

internal/pkg/api/handleCheckin.go:1562

• invalidKeyLRU.Store can exceed the configured maxBytes when maxBytes is set below invalidKeyLRUEntryBytes. In that case the eviction loop can’t remove anything (empty list) and the code will still insert one entry, violating the cap. Add a post-eviction guard so Store becomes a no-op when the cache can’t fit even a single entry.

	for c.used+invalidKeyLRUEntryBytes > c.maxBytes && c.l.Len() > 0 {
		c.evictOldest()
	}
	el := c.l.PushFront(&invalidKeyLRUEntry{agentID: agentID, state: s})
	c.items[agentID] = el
	c.used += invalidKeyLRUEntryBytes

internal/pkg/api/handleCheckin.go:231

• RunInvalidKeyStateCleaner starts a ticker and runs indefinitely even when graceful_force_unenroll is disabled. Since invalidKeyStates is only used when the feature is enabled (and the LRU is already size-bounded), this background loop is unnecessary work in the default configuration. Consider returning immediately when the flag is disabled (or only starting the goroutine when enabled).

func (ct *CheckinT) RunInvalidKeyStateCleaner(ctx context.Context) error {
	t := time.NewTicker(invalidKeyStateCleanInterval)
	defer t.Stop()
	for {

[blakerouse]

If we tell the agent to unenroll on a 401, and the root cause in ES is fixable (by revalidating the key, or maybe the ES failure is itself infrastructural and temporary), then we're forcing the user to manually re-enroll with a new key later, right? At the very least, there should be a delay on this, I think.

I am personally a bit hesitant to include UNENROLL in this for this reason, any kind of automatic unenroll we've ever introduced has lead to an incident of this sort. That said in this situation, the agents are already likely unmanaged and a users has opted into this, so it isn't automatic. If this weren't behind a feature flag it would be much more dangerous, though at the same time we could have people forget to turn it off and then get hit by this. We will eventually make unenroll continue to contact Fleet Server as part of the uninstall action anyway so maybe the unenroll has less value considering this, though until we do this work there is no other way to get agents to stop contacting fleet server.

If we want to cause this to be a little safer we could send 401 for the first hour, then only after a full hour of 401 then start the process of sending the empty policy, unenroll. That would allow the case for elasticsearch having interment issues for less than an hour to not be an issue, but if it went over an hour it would be the same result (which begs wether its worth doing at all then).

Do me this is a feature you turn on, leave on for 2 hours and then turn off. It should not be something that remains on, this is only an escape out of the mess and not the keep on all the time flow.

[cmacknz]

Do me this is a feature you turn on, leave on for 2 hours and then turn off. It should not be something that remains on, this is only an escape out of the mess and not the keep on all the time flow.

This is also how I am thinking about it.

[cmacknz]

Latest changes from my last review are just to the test, LGTM.

I still think we need an automated test of this with endpoint installed, probably on the agent side. That will likely be the most common use for this. I am particularly interested in making sure the unenroll action behaves reasonably since we know endpoint will reject it.