[Enhancement] Add structural limits for deserialization filter

[RyanL1997]

Description

Follow-up to #5469. Adds structural limits (maxdepth, maxrefs, maxbytes) to the filter created by DeserializationFilterUtil, bounding the size and shape of the deserialized object graph, and logs structural-limit rejections (depth/refs/bytes) alongside the existing class-rejection logging.

Related Issues

Follow-up to #5469

Check List

• New functionality includes testing.
• Commits are signed per the DCO using --signoff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 6143db0)

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 Structural limits order The structural limits are appended after additionalPatterns in the filter pattern string. If additionalPatterns contains a wildcard rejection (e.g., '!*'), it would block the structural limits from being parsed. While unlikely in typical usage, this could silently disable the intended protections if a caller passes such a pattern. String fullPattern = BASE_ALLOWLIST + additionalPatterns + STRUCTURAL_LIMITS + "!*";

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 6143db0

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Make structural limits configurable The structural limits may be too restrictive for legitimate use cases. Consider making these limits configurable through system properties or configuration files to allow tuning based on deployment requirements without code changes. core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java [50] -private static final String STRUCTURAL_LIMITS = "maxdepth=20;maxrefs=1000;maxbytes=15000;"; +private static final String STRUCTURAL_LIMITS = + String.format("maxdepth=%s;maxrefs=%s;maxbytes=%s;", + System.getProperty("opensearch.sql.deserialization.maxdepth", "20"), + System.getProperty("opensearch.sql.deserialization.maxrefs", "1000"), + System.getProperty("opensearch.sql.deserialization.maxbytes", "15000")); Suggestion importance[1-10]: 5 __ Why: The suggestion to make structural limits configurable is valid and could improve flexibility. However, the improved_code changes a static final String to be dynamically computed, which may have initialization order implications and doesn't match the original constant declaration pattern. The suggestion addresses a minor enhancement rather than a critical issue.	Low

---

Previous suggestions

Suggestions up to commit ec32115

Category	Suggestion	Impact
General	Make structural limits configurable The structural limits are hardcoded as constants. Consider making these configurable through system properties or configuration files to allow administrators to adjust limits based on their security requirements and application needs without code changes. core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java [50] -private static final String STRUCTURAL_LIMITS = "maxdepth=20;maxrefs=300;maxbytes=15000;"; +private static final int DEFAULT_MAX_DEPTH = 20; +private static final int DEFAULT_MAX_REFS = 300; +private static final int DEFAULT_MAX_BYTES = 15000; +private static final String STRUCTURAL_LIMITS = String.format( + "maxdepth=%d;maxrefs=%d;maxbytes=%d;", + Integer.getInteger("opensearch.sql.deserialization.maxdepth", DEFAULT_MAX_DEPTH), + Integer.getInteger("opensearch.sql.deserialization.maxrefs", DEFAULT_MAX_REFS), + Integer.getInteger("opensearch.sql.deserialization.maxbytes", DEFAULT_MAX_BYTES) +); + Suggestion importance[1-10]: 5 __ Why: While making security limits configurable could be useful for flexibility, this suggestion introduces complexity and potential security risks if misconfigured. The current hardcoded approach is acceptable for security-critical constants. The suggestion is valid but represents a design choice rather than a bug fix.	Low

Suggestions up to commit f73ca59

Category	Suggestion	Impact
Possible issue	Verify filter pattern ordering The order of pattern concatenation places structural limits after class patterns but before the reject-all pattern. Verify that this ordering is correct according to ObjectInputFilter specification, as structural limits typically should appear before class patterns. core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java [83] -String fullPattern = BASE_ALLOWLIST + additionalPatterns + STRUCTURAL_LIMITS + "!*"; +String fullPattern = STRUCTURAL_LIMITS + BASE_ALLOWLIST + additionalPatterns + "!*"; Suggestion importance[1-10]: 7 __ Why: This suggestion raises a valid concern about the ordering of filter patterns. According to ObjectInputFilter specification, structural limits should typically be placed before class patterns. This could affect the filter's behavior and should be verified.	Medium
General	Make structural limits configurable The structural limits may be too restrictive for legitimate use cases. Consider making these limits configurable through system properties or configuration files to allow adjustment based on deployment requirements without code changes. core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java [50] -private static final String STRUCTURAL_LIMITS = "maxdepth=20;maxrefs=300;maxbytes=15000;"; +private static final String STRUCTURAL_LIMITS = + "maxdepth=" + System.getProperty("opensearch.sql.deserialization.maxdepth", "20") + ";" + + "maxrefs=" + System.getProperty("opensearch.sql.deserialization.maxrefs", "300") + ";" + + "maxbytes=" + System.getProperty("opensearch.sql.deserialization.maxbytes", "15000") + ";"; Suggestion importance[1-10]: 5 __ Why: While making limits configurable adds flexibility, the suggestion is more of an enhancement than addressing a critical issue. The current hardcoded values are reasonable defaults, and the change adds complexity without evidence that configurability is needed.	Low

[github-actions]

Persistent review updated to latest commit ec32115

[github-actions]

Persistent review updated to latest commit 6143db0