Changing opensearch.cgroups.hierarchy.override causes java.lang.SecurityException exception by reta · Pull Request #20565 · opensearch-project/OpenSearch

reta · 2026-02-06T14:01:59Z

Description

At the moment, the permissions we grant by default apply to root of cgroup the hierarchy:

  permission java.io.FilePermission "/sys/fs/cgroup/memory.swap.max", "read";
  permission java.io.FilePermission "/sys/fs/cgroup/memory.swap.current", "read";
  permission java.io.FilePermission "/sys/fs/cgroup/memory.max", "read";
  permission java.io.FilePermission "/sys/fs/cgroup/memory.current", "read";

The opensearch.cgroups.hierarchy.override may change that and in general, the permissions become dynamic, based on the <cgroup name> name (whatever opensearch.cgroups.hierarchy.override points to):

  permission java.io.FilePermission "/sys/fs/cgroup/<cgroup name>/memory.swap.max", "read";
  permission java.io.FilePermission "/sys/fs/cgroup/<cgroup name>/memory.swap.current", "read";
  permission java.io.FilePermission "/sys/fs/cgroup/<cgroup name>/memory.max", "read";
  permission java.io.FilePermission "/sys/fs/cgroup/<cgroup name>/memory.current", "read";

The suggested fix it to support opensearch.cgroups.hierarchy.override expansion in policy files, so for example opensearch.cgroups.hierarchy.override=/test would translate to:

  permission java.io.FilePermission "/sys/fs/cgroup/test/memory.swap.max", "read";
  permission java.io.FilePermission "/sys/fs/cgroup/test/memory.swap.current", "read";
  permission java.io.FilePermission "/sys/fs/cgroup/test/memory.max", "read";
  permission java.io.FilePermission "/sys/fs/cgroup/test/memory.current", "read";

How to test / reproduce:

sudo mkdir /sys/fs/cgroup/test
sudo chmod --recursive o+rw /sys/fs/cgroup/test
OPENSEARCH_JAVA_OPTS="-Dopensearch.cgroups.hierarchy.override=/test" cgexec -g memory:test ./bin/opensearch

Related Issues

Resolves #20522

Check List

Functionality includes testing.
API changes companion pull request created, if applicable.
Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

coderabbitai · 2026-02-06T14:02:37Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

🔍 Trigger a full review

📝 Walkthrough

Walkthrough

This change introduces support for a new system property placeholder ${opensearch.cgroups.hierarchy.override} that can be dynamically substituted in security policy files. The implementation adds a helper method to PolicyFile for flexible placeholder expansion, extends the security policy to grant read permissions to cgroup memory paths using this placeholder, and includes corresponding tests to validate the functionality.

Changes

Cohort / File(s)	Summary
Documentation `CHANGELOG.md`	Adds changelog entry documenting that modifying opensearch.cgroups.hierarchy.override causes a java.lang.SecurityException.
Policy File Processing `libs/agent-sm/agent-policy/src/main/java/.../PolicyFile.java`	Introduces new private `expandSystemProperty()` helper method to support dynamic placeholder expansion with configurable separator handling. Extends `expandPermissionName()` to support `${opensearch.cgroups.hierarchy.override}` placeholder and updates path-terminator check logic.
Test Coverage `libs/agent-sm/agent-policy/src/test/java/.../PolicyFileTests.java`, `libs/agent-sm/agent-policy/src/test/resources/test.policy`	Adds new unit test class validating policy parsing and placeholder expansion, along with a test policy resource file granting cgroup and keystore file read permissions.
Security Policy Configuration `server/src/main/resources/.../security.policy`	Adds read permissions for cgroup memory-related paths using the new `${opensearch.cgroups.hierarchy.override}` placeholder variable (memory.swap.max, memory.swap.current, memory.max, memory.current, memory, and memory/\*).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Add file permissions for 'cpu,cpuacct' cgroup #20108: Modifies the same security.policy file to add FilePermission entries for additional cgroup paths (cpu/cpuacct and cpuset hierarchies), indicating ongoing cgroup-related security policy enhancements.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: adding support for opensearch.cgroups.hierarchy.override expansion in security policies to resolve SecurityException errors.
Description check	✅ Passed	The description includes a comprehensive explanation of the problem, proposed solution with examples, reproduction steps, and links to the related issue; all required sections from the template are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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

github-actions · 2026-02-06T14:05:22Z

❗ AI-powered Code-Diff-Analyzer found issues on commit 75aa263.

Path	Line	Severity	Description
libs/agent-sm/agent-policy/src/main/java/org/opensearch/secure_sm/policy/PolicyFile.java	146	medium	System property 'opensearch.cgroups.hierarchy.override' is expanded into file permission paths without validation against path traversal sequences (e.g., '../'). While this grants permissions rather than directly accessing files, a malicious value could grant overly broad read permissions outside the intended /sys/fs/cgroup/ directory. However, the code appears to be a legitimate feature for cgroup configuration rather than intentionally malicious, and exploiting this requires the ability to set system properties.

The table above displays the top 10 most important findings.

Total: 1 | Critical: 0 | High: 0 | Medium: 1 | Low: 0

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@libs/agent-sm/agent-policy/src/test/java/org/opensearch/secure_sm/policy/PolicyFileTests.java`:
- Around line 32-35: The test currently asserts an exact total count on
filePermissions (assertThat(filePermissions, hasSize(3))), which is brittle;
remove that exact size assertion and instead assert presence of the expected
entries only (use the existing checks on
filePermissions.stream().filter(...).count() or anyMatch to verify
"/sys/fs/cgroup/memory" and "/lib/security/cacerts" exist) so the test relies on
presence rather than an exact total; update PolicyFileTests.java to stop
asserting filePermissions size and keep/adjust the filtering assertions on
filePermissions to check existence (>=1) only.

🧹 Nitpick comments (1)

libs/agent-sm/agent-policy/src/main/java/org/opensearch/secure_sm/policy/PolicyFile.java (1)
146-163: Guard against unmatched placeholder patterns.

If a policy uses ${opensearch.cgroups.hierarchy.override} without both surrounding separators, index becomes -1 and the substring will throw. Consider an early guard or a more flexible replacement to avoid StringIndexOutOfBoundsException.
♻️ Suggested defensive guard
-        final int index = value.indexOf(separator + "${" + property + "}" + separator);
+        final String token = separator + "${" + property + "}" + separator;
+        final int index = value.indexOf(token);
+        if (index < 0) {
+            return value;
+        }
Also applies to: 175-180

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b6ab06b and b99e737.

📒 Files selected for processing (5)

CHANGELOG.md
libs/agent-sm/agent-policy/src/main/java/org/opensearch/secure_sm/policy/PolicyFile.java
libs/agent-sm/agent-policy/src/test/java/org/opensearch/secure_sm/policy/PolicyFileTests.java
libs/agent-sm/agent-policy/src/test/resources/test.policy
server/src/main/resources/org/opensearch/bootstrap/security.policy

🧰 Additional context used

🧬 Code graph analysis (1)

libs/agent-sm/agent-policy/src/test/java/org/opensearch/secure_sm/policy/PolicyFileTests.java (1)

libs/agent-sm/agent-policy/src/main/java/org/opensearch/secure_sm/policy/PolicyFile.java (1)

SuppressWarnings (43-393)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)

GitHub Check: gradle-check
GitHub Check: Analyze (java)
GitHub Check: assemble (21, ubuntu-24.04-arm)
GitHub Check: assemble (25, windows-latest)
GitHub Check: precommit (25, ubuntu-latest)
GitHub Check: precommit (21, ubuntu-latest)
GitHub Check: precommit (25, ubuntu-24.04-arm)
GitHub Check: precommit (21, macos-15)
GitHub Check: precommit (21, ubuntu-24.04-arm)
GitHub Check: precommit (25, windows-latest)
GitHub Check: precommit (21, windows-2025, true)
GitHub Check: precommit (25, macos-15-intel)
GitHub Check: precommit (21, windows-latest)
GitHub Check: assemble (21, windows-latest)
GitHub Check: assemble (25, ubuntu-latest)
GitHub Check: precommit (25, macos-15)
GitHub Check: assemble (21, ubuntu-latest)
GitHub Check: precommit (21, macos-15-intel)
GitHub Check: assemble (25, ubuntu-24.04-arm)

🔇 Additional comments (5)

CHANGELOG.md (1)

18-18: Changelog entry looks good.

Clear and appropriately placed under “Fixed.”

libs/agent-sm/agent-policy/src/test/resources/test.policy (1)

9-12: Looks good.

The policy entries align with the new placeholder expansion behavior.

libs/agent-sm/agent-policy/src/main/java/org/opensearch/secure_sm/policy/PolicyFile.java (1)

132-139: Good change using the correct path separator.

Using File.separator here fixes cross-platform path handling.

server/src/main/resources/org/opensearch/bootstrap/security.policy (1)

255-260: Looks good.

The new permissions correctly cover the override hierarchy paths.

libs/agent-sm/agent-policy/src/test/java/org/opensearch/secure_sm/policy/PolicyFileTests.java (1)

28-31: No action needed. The code is compatible with the project's target JDK version (Java 21). The PermissionCollection#elementsAsStream() method has been available since Java 9, and is therefore fully supported in Java 21.

Likely an incorrect or invalid review comment.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

libs/agent-sm/agent-policy/src/test/java/org/opensearch/secure_sm/policy/PolicyFileTests.java

github-actions · 2026-02-06T14:21:39Z

❌ Gradle check result for b99e737: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

…ityException exception Signed-off-by: Andriy Redko <drreta@gmail.com>

libs/agent-sm/agent-policy/src/main/java/org/opensearch/secure_sm/policy/PolicyFile.java

github-actions · 2026-02-06T15:47:22Z

❌ Gradle check result for d08131a: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2026-02-06T17:16:12Z

✅ Gradle check result for d08131a: SUCCESS

codecov · 2026-02-06T17:17:00Z

Codecov Report

❌ Patch coverage is 62.50000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.27%. Comparing base (3ba2f37) to head (e158cf7).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
...va/org/opensearch/secure_sm/policy/PolicyFile.java	62.50%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #20565      +/-   ##
============================================
+ Coverage     73.25%   73.27%   +0.02%     
+ Complexity    72103    72064      -39     
============================================
  Files          5798     5798              
  Lines        329732   329748      +16     
  Branches      47519    47523       +4     
============================================
+ Hits         241554   241633      +79     
+ Misses        68805    68732      -73     
- Partials      19373    19383      +10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

peterzhuamazon · 2026-02-06T17:34:46Z

Hi @reta do we need to update anything in https://github.com/peterzhuamazon/OpenSearch/blob/main/distribution/packages/src/common/systemd/opensearch.service ?

Thanks.

reta · 2026-02-06T17:50:18Z

Hi @reta do we need to update anything in https://github.com/peterzhuamazon/OpenSearch/blob/main/distribution/packages/src/common/systemd/opensearch.service ?

Thanks.

Thanks @peterzhuamazon , I believe manual changes will be needed there, since this is really a JVM property that we cannot use for systemd hardening, what do you think? The reporter on the issue mentioned it:

The paths for systemd cgroups are specified, but systemd cannot be used in a container.

Signed-off-by: Andriy Redko <drreta@gmail.com>

peterzhuamazon · 2026-02-06T22:43:43Z

Hi @reta do we have any defaults for opensearch.cgroups.hierarchy.override? I can definitely update that in opensearch.service to start with. Else I can keep it for now and check back next release to see if anything blocked.

For deb/rpm it is not as flexible as tar, so I think we can hardcode the default into systemd file for now.

reta · 2026-02-06T22:49:08Z

Hi @reta do we have any defaults for opensearch.cgroups.hierarchy.override? I can definitely update that in opensearch.service to start with. Else I can keep it for now and check back next release to see if anything blocked.

@peterzhuamazon Yes, default is / which is what we need and fits default settings perfectly (no issues). Basically, we don't need any changes for default settings.

github-actions · 2026-02-06T23:02:50Z

✅ Gradle check result for e158cf7: SUCCESS

…ityException exception (opensearch-project#20565) * Changing opensearch.cgroups.hierarchy.override causes java.lang.SecurityException exception Signed-off-by: Andriy Redko <drreta@gmail.com> * Address code review comments Signed-off-by: Andriy Redko <drreta@gmail.com> --------- Signed-off-by: Andriy Redko <drreta@gmail.com>

reta requested a review from a team as a code owner February 6, 2026 14:02

reta added the skip-changelog label Feb 6, 2026

github-actions bot added bug Something isn't working Other v3.6.0 Issues and PRs related to version 3.6.0 labels Feb 6, 2026

reta force-pushed the issue-20522 branch from 31852b3 to 75aa263 Compare February 6, 2026 14:03

reta removed the skip-changelog label Feb 6, 2026

reta force-pushed the issue-20522 branch from 75aa263 to b99e737 Compare February 6, 2026 14:07

coderabbitai bot reviewed Feb 6, 2026

View reviewed changes

libs/agent-sm/agent-policy/src/test/java/org/opensearch/secure_sm/policy/PolicyFileTests.java Outdated Show resolved Hide resolved

Changing opensearch.cgroups.hierarchy.override causes java.lang.Secur…

d08131a

…ityException exception Signed-off-by: Andriy Redko <drreta@gmail.com>

reta force-pushed the issue-20522 branch from b99e737 to d08131a Compare February 6, 2026 14:27

cwperks reviewed Feb 6, 2026

View reviewed changes

libs/agent-sm/agent-policy/src/main/java/org/opensearch/secure_sm/policy/PolicyFile.java Show resolved Hide resolved

cwperks approved these changes Feb 6, 2026

View reviewed changes

cwperks added backport 3.5 Backport to 3.5 branch and removed backport 3.5 Backport to 3.5 branch labels Feb 6, 2026

Address code review comments

e158cf7

Signed-off-by: Andriy Redko <drreta@gmail.com>

cwperks approved these changes Feb 6, 2026

View reviewed changes

reta merged commit 985b0c8 into opensearch-project:main Feb 7, 2026
34 of 35 checks passed

opensearch-ci-bot mentioned this pull request Feb 18, 2026

[AUTOCUT] Gradle Check Flaky Test Report for PruneFileCacheIT #19724

Open

This was referenced Feb 18, 2026

[AUTOCUT] Gradle Check Flaky Test Report for RemoteRoutingTableServiceTests #14559

Open

[AUTOCUT] Gradle Check Flaky Test Report for SegmentReplicationSuiteIT #19258

Open

[AUTOCUT] Gradle Check Flaky Test Report for ClusterDisruptionIT #14308

Open

Conversation

reta commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

coderabbitai bot commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

github-actions bot commented Feb 6, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Feb 6, 2026

Uh oh!

Uh oh!

github-actions bot commented Feb 6, 2026

Uh oh!

github-actions bot commented Feb 6, 2026

Uh oh!

codecov bot commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

peterzhuamazon commented Feb 6, 2026

Uh oh!

reta commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peterzhuamazon commented Feb 6, 2026

Uh oh!

reta commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

reta commented Feb 6, 2026 •

edited

Loading

coderabbitai bot commented Feb 6, 2026 •

edited

Loading

codecov bot commented Feb 6, 2026 •

edited

Loading

reta commented Feb 6, 2026 •

edited

Loading

reta commented Feb 6, 2026 •

edited

Loading