Update logic in FipsBuildParams.isInFipsApprovedOnlyMode to check for env param instead of org.bouncycastle.fips.approved_only

[cwperks]

Description

Currently, FipsBuildParam looks for the system prop (org.bouncycastle.fips.approved_only) to determine if FIPS is enforced at runtime. This system prop is specific to the bouncycastle library and implicitly set here in opensearch-env. This PR makes configuring FIPS more intentional by the cluster operator by looking for an OpenSearch env var instead of the bouncycastle system prop to determine if FIPS should be enforced at runtime.

Related Issues

Related to #20738

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.

[cwperks]

@beanuwave @terryquigleysas here's another PR from our discussion last week. Let me know what you think.

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 3b50892.

Path	Line	Severity	Description
buildSrc/src/main/java/org/opensearch/gradle/info/FipsBuildParams.java	22	medium	isInFipsApprovedOnlyMode() was changed from reading a JVM system property ('org.bouncycastle.fips.approved_only') to reading an environment variable ('OPENSEARCH_FIPS_MODE'). This alters the FIPS approved-only mode activation mechanism in a non-obvious way: environment variables are generally easier to set/override in CI/CD and container environments than JVM properties, potentially making it easier to accidentally or intentionally enable/disable this security-critical mode. The package-private (non-private) supplier field also allows test and same-package code to override the env-reading behavior at runtime.

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.

[reta]

Thanks @cwperks !

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Package-Private Field The fipsModeEnvSupplier field is package-private (no access modifier), which exposes it for mutation from outside the class. While this is intentional for testing, it could be accidentally modified in production code. Consider using a private field with a package-private or protected setter, or using a different injection mechanism to limit exposure. static Supplier<String> fipsModeEnvSupplier = () -> System.getenv("OPENSEARCH_FIPS_MODE"); Test State Leak The test manually resets fipsModeEnvSupplier at the end, but if any assertion fails before the reset, the supplier will remain in a modified state and could affect subsequent tests. Consider using a try/finally block or a setup/teardown method to ensure the supplier is always reset. // Reset FipsBuildParams.fipsModeEnvSupplier = () -> System.getenv("OPENSEARCH_FIPS_MODE");

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Ensure test state is always reset Resetting shared static state only at the end of the test method is fragile — if any assertion fails, the reset is skipped, potentially polluting other tests. Use a tearDown method (or @After) to guarantee the supplier is always restored regardless of test outcome. buildSrc/src/test/java/org/opensearch/gradle/info/FipsBuildParamsTests.java [37-38] -// Reset -FipsBuildParams.fipsModeEnvSupplier = () -> System.getenv("OPENSEARCH_FIPS_MODE"); +@Override +protected void tearDown() throws Exception { + super.tearDown(); + FipsBuildParams.fipsModeEnvSupplier = () -> System.getenv("OPENSEARCH_FIPS_MODE"); +} Suggestion importance[1-10]: 6 __ Why: This is a valid concern — if an assertion fails mid-test, the fipsModeEnvSupplier reset at the end of the method is skipped, potentially causing test pollution. Moving the reset to tearDown is a standard JUnit pattern that ensures cleanup regardless of test outcome.	Low
General	Restrict mutable supplier field visibility The fipsModeEnvSupplier field has package-private (default) visibility, which exposes it to mutation from any class in the same package, not just tests. Consider making it private and providing a package-private or test-only setter, or using a visibility annotation to limit unintended access in production code. buildSrc/src/main/java/org/opensearch/gradle/info/FipsBuildParams.java [22] -static Supplier<String> fipsModeEnvSupplier = () -> System.getenv("OPENSEARCH_FIPS_MODE"); +private static Supplier<String> fipsModeEnvSupplier = () -> System.getenv("OPENSEARCH_FIPS_MODE"); +static void setFipsModeEnvSupplierForTesting(Supplier<String> supplier) { + fipsModeEnvSupplier = supplier; +} + Suggestion importance[1-10]: 5 __ Why: The fipsModeEnvSupplier field is intentionally package-private to allow direct mutation in tests (as seen in FipsBuildParamsTests). Making it private with a setter would improve encapsulation, but the suggested improved_code introduces a new method that would require updating the test file as well, making this a moderate refactoring suggestion rather than a critical fix.	Low

[github-actions]

✅ Gradle check result for 3b50892: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.34%. Comparing base (436e4a6) to head (3b50892).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #21366      +/-   ##
============================================
- Coverage     73.34%   73.34%   -0.01%     
+ Complexity    74223    74204      -19     
============================================
  Files          5958     5958              
  Lines        337309   337359      +50     
  Branches      48664    48687      +23     
============================================
+ Hits         247408   247432      +24     
+ Misses        70188    70169      -19     
- Partials      19713    19758      +45     

☔ 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.