Skip to content

Check hidden frames in entitlements #127877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 8, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/127877.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 127877
summary: Check hidden frames in entitlements
area: Infra/Entitlements
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to change this to Infra/Core or add Infra/Entitlements to the changelog schema

type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@
package org.elasticsearch.entitlement.bridge;

import java.util.Optional;
import java.util.Set;

import static java.lang.StackWalker.Option.RETAIN_CLASS_REFERENCE;
import static java.lang.StackWalker.Option.SHOW_HIDDEN_FRAMES;

public class Util {
/**
Expand All @@ -23,6 +25,8 @@ public class Util {
public static final Class<?> NO_CLASS = new Object() {
}.getClass();

private static final Set<String> skipInternalPackages = Set.of("java.lang.invoke", "java.lang.reflect", "jdk.internal.reflect");
Copy link
Contributor

Choose a reason for hiding this comment

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

This set leaves me a bit uneasy -- we should at least test this is complete for all JDKs we support

Copy link
Member Author

Choose a reason for hiding this comment

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

This will be difficult to do ahead of time without a lot of work. I'm going to leave this up to our jdk matrix tests that are run on main, and will watch that job.


/**
* Why would we write this instead of using {@link StackWalker#getCallerClass()}?
* Because that method throws {@link IllegalCallerException} if called from the "outermost frame",
Expand All @@ -32,9 +36,10 @@ public class Util {
*/
@SuppressWarnings("unused") // Called reflectively from InstrumenterImpl
public static Class<?> getCallerClass() {
Optional<Class<?>> callerClassIfAny = StackWalker.getInstance(RETAIN_CLASS_REFERENCE)
Optional<Class<?>> callerClassIfAny = StackWalker.getInstance(Set.of(RETAIN_CLASS_REFERENCE, SHOW_HIDDEN_FRAMES))
.walk(
frames -> frames.skip(2) // Skip this method and its caller
.filter(frame -> skipInternalPackages.contains(frame.getDeclaringClass().getPackageName()) == false)
.findFirst()
.map(StackWalker.StackFrame::getDeclaringClass)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,9 @@ private static BaseDir parseBaseDir(String baseDir) {
case "config" -> BaseDir.CONFIG;
case "data" -> BaseDir.DATA;
case "home" -> BaseDir.USER_HOME;
// NOTE: shared_repo is _not_ accessible to policy files, only internally
// it would be nice to limit this to just ES modules, but we don't have a way to plumb that through to here
// however, we still don't document in the error case below that shared_repo is valid
case "shared_repo" -> BaseDir.SHARED_REPO;
default -> throw new PolicyValidationException(
"invalid relative directory: " + baseDir + ", valid values: [config, data, home]"
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,8 @@
org.elasticsearch.repository.url:
- outbound_network
- files:
- relative_path: .
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit/comment: elsewhere we expressed this as "", but I actually like '.' more

relative_to: shared_repo
mode: read
org.apache.httpcomponents.httpclient:
- outbound_network # for URLHttpClient
Loading