Skip to content

fix: enforce policy-based access control on artifact downloads#7009

Merged
ycombinator merged 19 commits into
elastic:mainfrom
ycombinator:fix/artifact-access-control
Jun 5, 2026
Merged

fix: enforce policy-based access control on artifact downloads#7009
ycombinator merged 19 commits into
elastic:mainfrom
ycombinator:fix/artifact-access-control

Conversation

@ycombinator

@ycombinator ycombinator commented May 11, 2026

Copy link
Copy Markdown
Contributor

What is the problem this PR solves?

The artifact download endpoint (/api/fleet/artifacts/{id}/{sha256}) only validates the agent's API key but never checks whether the requested artifact belongs to the agent's assigned policy. This means an agent enrolled under one policy can download artifacts belonging to a different policy if it knows the artifact ID and SHA256 hash. For example, an agent enrolled under a policy with no integrations can retrieve Elastic Defend trust lists, exception lists, and other security artifacts from another policy.

How does this PR solve the problem?

Implements the authorizeArtifact() function (previously a no-op that returned nil) to enforce policy-based access control:

  1. Adds a GetPolicy(ctx, policyID) method to the policy.Monitor interface that returns the cached policy for a given ID (reloads from ES on cache miss).
  2. In authorizeArtifact, fetches the agent's policy via the monitor using agent.AgentPolicyID and verifies that the requested artifact (identifier + decoded_sha256) appears in the policy's inputs[].artifact_manifest.artifacts.
  3. Returns 403 Forbidden (ErrUnauthorizedArtifact) if the artifact is not listed in the agent's assigned policy.

How to test this PR locally

  1. Set up Fleet Server with Elasticsearch and Kibana
  2. Create two agent policies: Victim-Policy with Elastic Defend integration (add a trusted application), and Attacker-Policy with no integrations
  3. Create an enrollment token for Attacker-Policy and enroll an agent
  4. Attempt to download an artifact belonging to Victim-Policy using the attacker agent's API key — should now receive 403 Forbidden instead of the artifact contents
  5. Verify that an agent enrolled under Victim-Policy can still download its own artifacts normally (200 OK)

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 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

@ycombinator ycombinator requested a review from a team as a code owner May 11, 2026 22:44
@ycombinator ycombinator requested review from macdewee and samuelvl May 11, 2026 22:44
@mergify

mergify Bot commented May 11, 2026

Copy link
Copy Markdown
Contributor

This pull request does not have a backport label. Could you fix it @ycombinator? 🙏
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.

@ycombinator ycombinator requested review from michel-laterman and samuelvl and removed request for macdewee and samuelvl May 11, 2026 22:52
@ycombinator ycombinator added bug Something isn't working Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team backport-active-all Automated backport with mergify to all the active branches labels May 11, 2026
@ycombinator ycombinator requested review from cmacknz and kruskall May 11, 2026 23:09
Comment thread internal/pkg/api/handleArtifacts.go Outdated
Comment thread internal/pkg/api/handleArtifacts.go Outdated
@github-actions

This comment has been minimized.

@ycombinator ycombinator enabled auto-merge (squash) May 12, 2026 19:50

@michel-laterman michel-laterman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Comment thread internal/pkg/api/handleArtifacts.go Outdated
@ycombinator ycombinator force-pushed the fix/artifact-access-control branch from 2e1103d to 535b14f Compare May 15, 2026 13:42
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@michel-laterman michel-laterman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, just have nitpicks

Comment thread testing/e2e/api_version/client_api_current.go Outdated
Comment thread testing/e2e/api_version/client_api_2023_06_01.go Outdated
ycombinator and others added 12 commits June 4, 2026 06:22
schema.go is code-generated and gets overwritten by mage generate.
Moving ArtifactManifest and ManifestEntry to ext.go keeps them stable.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ArtifactManifest and ManifestEntry are not ES document types and only
exist to support parsing within handleArtifacts.go, so they belong there
rather than in the model package.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Agents enrolled under dummy-policy cannot download Elastic Defend
artifacts because that policy has no artifact_manifest. Enroll the test
agent under security-policy (which has the Elastic Defend integration)
instead.

Add FleetPolicyHasArtifact scaffold helper that polls .fleet-policies
until the policy document references the artifact, ensuring fleet-server's
policy monitor cache is up-to-date before the download attempt. Also
retry the download on 403 to tolerate any remaining cache propagation lag.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…checks

PolicyData.Inputs is already decoded as []map[string]any, so marshaling
back to JSON just to unmarshal again is unnecessary. Use type assertions
directly on the decoded map in both policyHasArtifact (production) and
policyInputHasArtifact (e2e scaffold), removing the artifactManifest and
manifestEntry types along with parseArtifactManifest.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ntext

The ES global checkpoint monitor can hold up to 4 minutes before the
policy cache refreshes. On slow CI the setup steps (FleetHasArtifacts +
FleetPolicyHasArtifact) could exhaust the 3-minute budget, leaving the
retry loop to fail with a misleading "context deadline exceeded" from the
HTTP call. Raise the budget to 5 minutes and add an explicit ctx.Err()
check at the top of the retry loop so expiry surfaces a clear message.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…acts

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ycombinator ycombinator force-pushed the fix/artifact-access-control branch from 9d7fccf to 5e5192b Compare June 4, 2026 13:23
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

TL;DR

The Buildkite failure is a hung E2E test in TestOpAMPWithUpstreamCollector, not a compile error. The test blocks in a deferred otelCmd.Wait() and eventually the package-level test timeout aborts the run.

Remediation

  • Add a hard stop for the collector process in testing/e2e/stand_alone_test.go after graceful shutdown is attempted (for example: set otelCmd.WaitDelay and/or fall back to Process.Kill if SIGTERM does not exit promptly).
  • Re-run the E2E step after updating process shutdown handling, and specifically verify TestStandAloneRunningSuite/TestOpAMPWithUpstreamCollector exits cleanly.
Investigation details

Root Cause

The stack trace in the failed job shows the test goroutine waiting in os/exec.(*Cmd).Wait from TestOpAMPWithUpstreamCollector:

  • testing/e2e/stand_alone_test.go:712 (defer otelCmd.Wait())
  • function returns at testing/e2e/stand_alone_test.go:729, where the deferred wait executes.

The collector command is started with context cancellation and a custom cancel handler:

  • testing/e2e/stand_alone_test.go:703-706 sets otelCmd.Cancel to only send SIGTERM.

If the child process does not terminate promptly on SIGTERM, deferred Wait() can block and the suite eventually times out, which matches the Buildkite log (long-running goroutine blocked in Cmd.Wait, package fails after ~1980s).

Evidence

  • Build: https://buildkite.com/elastic/fleet-server/builds/14999
  • Job/step: E2E Test (.buildkite/scripts/e2e_test.sh)
  • Key log excerpt:
    • github.com/elastic/fleet-server/testing/e2e.(*StandAloneSuite).TestOpAMPWithUpstreamCollector
    • /opt/buildkite-agent/.../stand_alone_test.go:729 +0x18de
    • os/exec.(*Cmd).Wait(...)/src/os/exec/exec.go:930
    • FAIL github.com/elastic/fleet-server/testing/e2e 1980.062s

Related flaky-test tracking: #6590 (same test area under stand_alone_test.go, previously reported instability).

Verification

  • Not run locally (requires full E2E environment and external dependency build path in CI).

Follow-up

If this test still flakes after adding a forced-stop path, consider explicitly asserting collector shutdown completion (with a bounded timeout) so hangs fail fast with a direct assertion instead of package timeout.

Note

🔒 Integrity filter blocked 2 items

The following items were blocked because they don't meet the GitHub integrity level.

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

What is this? | From workflow: PR Buildkite Detective

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

agent_policy_id is only populated after first checkin; policy_id is set
at enrollment. Use policy_id as fallback so newly enrolled agents can
download artifacts before their first checkin.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ycombinator ycombinator merged commit caa8b2d into elastic:main Jun 5, 2026
10 checks passed
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

@Mergifyio backport 9.4 9.3 8.19

@mergify

mergify Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

backport 9.4 9.3 8.19

✅ Backports have been created

Details

Cherry-pick of caa8b2d has failed:

On branch mergify/bp/8.19/pr-7009
Your branch is up to date with 'origin/8.19'.

You are currently cherry-picking commit caa8b2d.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   changelog/fragments/1778540235-fix-artifact-access-control.yaml
	modified:   internal/pkg/api/error.go
	modified:   internal/pkg/api/handleArtifacts.go
	new file:   internal/pkg/api/handleArtifacts_test.go
	modified:   internal/pkg/dl/constants.go
	modified:   internal/pkg/server/fleet.go
	modified:   testing/e2e/api_version/client_api_2023_06_01.go
	modified:   testing/e2e/api_version/client_api_current.go
	modified:   testing/e2e/scaffold/scaffold.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   internal/pkg/api/handleCheckin_test.go
	both modified:   internal/pkg/policy/monitor.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-active-all Automated backport with mergify to all the active branches bug Something isn't working Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants