Skip to content
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