Skip to content

Conversation

@jdconrad
Copy link
Contributor

This prevents an incorrect binary search by pruning files entitlements paths at the time of construction.

As an example:

/foo [read]
/foo/bar [read]

look up
/foo/baz [read]

This would previously cause /foo/baz to see /foo/bar as its parent instead of /foo.

This change prunes /foo/bar from the read list since /foo is already available. And now foo/baz will see foo as its parent.

@jdconrad jdconrad added >bug auto-backport Automatically create backport pull requests when merged test-entitlements v8.18.1 v8.19.0 v9.0.1 v9.1.0 :Core/Infra/Entitlements Entitlements infrastructure labels Feb 21, 2025
@jdconrad jdconrad requested a review from a team as a code owner February 21, 2025 18:11
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Feb 21, 2025
@elasticsearchmachine
Copy link
Collaborator

Hi @jdconrad, I've created a changelog YAML for you.

@jdconrad jdconrad added >non-issue and removed >bug labels Feb 21, 2025
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

assertThat(tree.canRead(path("foo/bar")), is(true));
assertThat(tree.canWrite(path("foo/bar")), is(false));
assertThat(tree.canRead(path("foo/baz")), is(true));
assertThat(tree.canWrite(path("foo/baz")), is(false));
Copy link
Member

Choose a reason for hiding this comment

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

Can we test a path has read access that is not explicitly entitled? ie that would be hit by the bug

this.writePaths = pruneSortedPaths(writePaths).toArray(new String[0]);
}

public static List<String> pruneSortedPaths(List<String> paths) {
Copy link
Member

Choose a reason for hiding this comment

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

this can be private?

@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.18
8.x
9.0

jdconrad added a commit to jdconrad/elasticsearch that referenced this pull request Feb 22, 2025
jdconrad added a commit to jdconrad/elasticsearch that referenced this pull request Feb 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Core/Infra/Entitlements Entitlements infrastructure >non-issue Team:Core/Infra Meta label for core/infra team v8.18.1 v8.19.0 v9.0.1 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants