Skip to content

Remove Security Manager usage from x-pack ml#145043

Open
jdconrad wants to merge 3 commits intoelastic:mainfrom
jdconrad:remove-xpack-ml-security-manager
Open

Remove Security Manager usage from x-pack ml#145043
jdconrad wants to merge 3 commits intoelastic:mainfrom
jdconrad:remove-xpack-ml-security-manager

Conversation

@jdconrad
Copy link
Contributor

@jdconrad jdconrad commented Mar 26, 2026 •
edited
Loading

Removes all AccessController.doPrivileged, PrivilegedAction, and SecurityManager usage from the x-pack ml plugin.

ES-14393

jdconrad and others added 2 commits March 24, 2026 13:51
@jdconrad @claude
Remove Security Manager usage from x-pack ml
0b7ed35 
- NamedPipeHelper: remove SpecialPermission.check() guards and
  AccessController.doPrivileged wrappers around pipe open operations;
  drop PrivilegedAction from inner helper classes
- DomainSplitFunction: inline two doPrivileged calls — one wrapping
  resource file loading in static init, one wrapping a deprecation
  warning emitted from script context
- LinearProgrammingPlanSolver: inline privilegedModelMaximise() and
  remove doPrivileged wrapper around ojalgo model.maximise(); remove
  @SuppressWarnings("removal") annotation

SecurityManager was removed in JDK 17 (JEP 411) and AccessController
is deprecated for removal since JDK 17. All of this code was dead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jdconrad @claude
Simplify NamedPipeHelper by removing propagateWrappedException
746c611 
Now that FileInputStream/FileOutputStream are called directly (no longer
wrapped in AccessController.doPrivileged), IOException is caught directly
in the retry loops and can be rethrown as-is. The wrapping in
RuntimeException and the propagateWrappedException unwrapper are no
longer needed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@elasticsearchmachine elasticsearchmachine added v9.4.0 needs:triage Requires assignment of a team area label labels Mar 26, 2026
@jdconrad jdconrad added >refactoring Team:Core/Infra Meta label for core/infra team :Core/Infra/Entitlements Entitlements infrastructure branch:9.2 branch:8.19 branch:9.3 auto-backport Automatically create backport pull requests when merged and removed needs:triage Requires assignment of a team area label labels Mar 26, 2026
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Mar 26, 2026

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 5055b17e-ea0e-425d-b249-368cce048bd0

📥 Commits

Reviewing files that changed from the base of the PR and between fab219c and 746c611.

📒 Files selected for processing (3)
  • x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/assignment/planning/LinearProgrammingPlanSolver.java
  • x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/DomainSplitFunction.java
  • x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/NamedPipeHelper.java
📝 Walkthrough

Walkthrough

Removed Java security context wrapping and privilege elevation logic from three ML utility and planning modules. The changes eliminate AccessController.doPrivileged() calls and related security scaffolding in linear programming solver operations, domain name splitting, and named pipe handling. Direct method calls now replace the privileged action wrappers. The named pipe helper adds @SuppressForbidden annotations to accommodate direct stream creation without privilege wrapping. Error handling and control flow remain functionally intact.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • 🛠️ Update Documentation: Commit on current branch
  • 🛠️ Update Documentation: Create PR

Comment @coderabbitai help to get the list of available commands and usage tips.

mark-vieira
mark-vieira reviewed Mar 26, 2026
View reviewed changes
x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/NamedPipeHelper.java
} catch (InterruptedException ie) {
Thread.currentThread().interrupt();
propagatePrivilegedException(e);
throw e;
Copy link
Contributor

@mark-vieira mark-vieira Mar 26, 2026

Choose a reason for hiding this comment

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

Was this a bug to begin with? Do we want to throw the outer exception here or the InterruptedException?

Copy link
Member

@rjernst rjernst Mar 26, 2026

Choose a reason for hiding this comment

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

If there are multiple exceptions let's make sure to use addSuppressed to not lose it completely

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

Reviewers

@rjernst rjernst rjernst left review comments

@mark-vieira mark-vieira mark-vieira left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

auto-backport Automatically create backport pull requests when merged :Core/Infra/Entitlements Entitlements infrastructure >refactoring Team:Core/Infra Meta label for core/infra team v8.19.14 v9.2.8 v9.3.3 v9.4.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jdconrad @elasticsearchmachine @rjernst @mark-vieira