fix: enforce policy-based access control on artifact downloads#7009
Conversation
|
This pull request does not have a backport label. Could you fix it @ycombinator? 🙏
|
This comment has been minimized.
This comment has been minimized.
2e1103d to
535b14f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
michel-laterman
left a comment
There was a problem hiding this comment.
lgtm, just have nitpicks
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>
9d7fccf to
5e5192b
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
TL;DRThe Buildkite failure is a hung E2E test in Remediation
Investigation detailsRoot CauseThe stack trace in the failed job shows the test goroutine waiting in
The collector command is started with context cancellation and a custom cancel handler:
If the child process does not terminate promptly on Evidence
Related flaky-test tracking: #6590 (same test area under Verification
Follow-upIf 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 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | noneWhat 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>
|
@Mergifyio backport 9.4 9.3 8.19 |
✅ Backports have been createdDetails
Cherry-pick of caa8b2d has failed: 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 |
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 returnednil) to enforce policy-based access control:GetPolicy(ctx, policyID)method to thepolicy.Monitorinterface that returns the cached policy for a given ID (reloads from ES on cache miss).authorizeArtifact, fetches the agent's policy via the monitor usingagent.AgentPolicyIDand verifies that the requested artifact (identifier+decoded_sha256) appears in the policy'sinputs[].artifact_manifest.artifacts.ErrUnauthorizedArtifact) if the artifact is not listed in the agent's assigned policy.How to test this PR locally
Design Checklist
Checklist
./changelog/fragmentsusing the changelog tool