[CLEAN] Synthetic Benchmark PR #137712 - Add User Profile Size Limit Enforced During Profile Updates

[tomerqodo]

User description

Benchmark PR elastic#137712

Type: Clean (correct implementation)

Original PR Title: Add User Profile Size Limit Enforced During Profile Updates
Original PR Description: Currently, there are no limits on the size of a user profile. Profiles store username, initials, avatars, etc.

Authorized Kibana observability clients can store an unlimited amount of data in user profile via update-profile.

This change puts a limit on profile size to avoid heap memory pressure and OOM crashes.

A new configuration setting, xpack.security.profile.max_size, was introduced with a default value of 10 MB to remain safely above the 1 MB request limit size enforced by Kibana.

Limit enforcement is implemented with a profile document read before the update, to provide a full view of the profile footprint. This approach is intended to be lightweight. Still, a document read is now incurred for every update request.
Original PR URL: elastic#137712

---

PR Type

Enhancement

---

Description

• Add configurable user profile size limit to prevent heap memory pressure

• Implement profile size validation before update operations

• Introduce xpack.security.profile.max_size setting with 10 MB default

• Add comprehensive tests for profile size quota enforcement

---

Diagram Walkthrough

flowchart LR
  A["Profile Update Request"] --> B["Fetch Current Profile"]
  B --> C["Validate Combined Size"]
  C --> D{Size within Limit?}
  D -->|Yes| E["Execute Update"]
  D -->|No| F["Reject with Error"]
  E --> G["Update Successful"]
  F --> H["BAD_REQUEST Exception"]

Loading

File Walkthrough

	Relevant files
Enhancement	ProfileService.java Implement profile size validation and configuration            x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/profile/ProfileService.java Add MAX_SIZE_SETTING configuration for profile size limit (default 10 MB) Store maxProfileSize as instance variable initialized from settings Implement validateProfileSize() method to check combined profile size against limit Add helper methods: combineMaps(), mapFromBytesReference(), serializationSize() Modify updateProfileData() to fetch current profile and validate size before update +67/-4
Configuration changes	Security.java Register profile size limit setting                                            x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java Register ProfileService.MAX_SIZE_SETTING in the security settings list +1/-0
Tests	ProfileServiceTests.java Add unit tests for profile size validation                              x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/profile/ProfileServiceTests.java Add unit tests for serializationSize() method with various map inputs Add unit tests for mapFromBytesReference() with null and JSON inputs Add unit tests for combineMaps() with nested map merging Add unit tests for validateProfileSize() with size limit enforcement +54/-0    ProfileIntegTests.java Add integration test for profile quota enforcement              x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/profile/ProfileIntegTests.java Set profile size limit to 1 KB in test node settings Add integration test testUpdateProfileDataHitStorageQuota() to verify quota enforcement Test scenario: fill half quota, update in-place, then overflow with new key +33/-0
Documentation	137712.yaml Add changelog entry for profile size limit                              docs/changelog/137712.yaml Add changelog entry documenting the profile size limit feature Mark as bug fix type in Security area +5/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit logs: The new critical action of rejecting profile updates due to size limits does not add any audit logging of the attempt, user, or outcome. Referred Code static void validateProfileSize(VersionedDocument doc, UpdateProfileDataRequest request, ByteSizeValue limit) { if (doc == null) { return; } Map<String, Object> labels = combineMaps(doc.doc.labels(), request.getLabels()); Map<String, Object> data = combineMaps(mapFromBytesReference(doc.doc.applicationData()), request.getData()); ByteSizeValue actualSize = ByteSizeValue.ofBytes(serializationSize(labels) + serializationSize(data)); if (actualSize.compareTo(limit) > 0) { throw new ElasticsearchStatusException( Strings.format( "cannot update profile [%s] because the combined profile size of [%s] exceeds the maximum of [%s]", request.getUid(), actualSize, limit ), RestStatus.BAD_REQUEST ); } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: No log on reject: The rejection path for oversized profile updates throws an error without any structured log, reducing observability while not leaking sensitive data. Referred Code static void validateProfileSize(VersionedDocument doc, UpdateProfileDataRequest request, ByteSizeValue limit) { if (doc == null) { return; } Map<String, Object> labels = combineMaps(doc.doc.labels(), request.getLabels()); Map<String, Object> data = combineMaps(mapFromBytesReference(doc.doc.applicationData()), request.getData()); ByteSizeValue actualSize = ByteSizeValue.ofBytes(serializationSize(labels) + serializationSize(data)); if (actualSize.compareTo(limit) > 0) { throw new ElasticsearchStatusException( Strings.format( "cannot update profile [%s] because the combined profile size of [%s] exceeds the maximum of [%s]", request.getUid(), actualSize, limit ), RestStatus.BAD_REQUEST ); } Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consider using a scripted update Instead of reading the profile document before each update, use a scripted update. This combines size validation and the update into a single atomic operation, improving performance by avoiding an extra network round trip. Examples: x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/profile/ProfileService.java [234-266] public void updateProfileData(UpdateProfileDataRequest request, ActionListener<AcknowledgedResponse> listener) { final XContentBuilder builder; try { builder = XContentFactory.jsonBuilder(); builder.startObject(); { builder.field("user_profile"); builder.startObject(); { if (false == request.getLabels().isEmpty()) { ... (clipped 23 lines) Solution Walkthrough: Before: public void updateProfileData(UpdateProfileDataRequest request, ActionListener<...> listener) { // 1. Fetch the current document from Elasticsearch getVersionedDocument(request.getUid(), ActionListener.wrap(doc -> { // 2. Validate the size locally validateProfileSize(doc, request, maxProfileSize); // 3. Build and send a separate update request to Elasticsearch UpdateRequest updateRequest = buildUpdateRequest(...); doUpdate( updateRequest, listener.map(...) ); }, listener::onFailure)); } After: public void updateProfileData(UpdateProfileDataRequest request, ActionListener<...> listener) { // 1. Create a script to perform validation and update on the data node Script script = new Script( ScriptType.INLINE, "painless", "// Logic to combine ctx._source with params, calculate size, and update if valid", Map.of("update", request.getData(), "limit", maxProfileSize.getBytes()) ); // 2. Build a single scripted update request UpdateRequest updateRequest = new UpdateRequest(SECURITY_PROFILE_ALIAS, uidToDocId(request.getUid())) .script(script) .setRefreshPolicy(request.getRefreshPolicy()); // 3. Execute the single atomic operation doUpdate(updateRequest, listener.map(...)); } Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies the performance overhead of the read-then-write pattern and proposes a more efficient, atomic, and idiomatic Elasticsearch solution using a scripted update.	High
Possible issue	Fix inaccurate profile size calculation Fix the inaccurate profile size calculation in validateProfileSize by serializing the labels and application_data maps within a structure that mirrors the final document, including the field names, to get a correct size. x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/profile/ProfileService.java [268-286] static void validateProfileSize(VersionedDocument doc, UpdateProfileDataRequest request, ByteSizeValue limit) { if (doc == null) { return; } Map<String, Object> labels = combineMaps(doc.doc.labels(), request.getLabels()); Map<String, Object> data = combineMaps(mapFromBytesReference(doc.doc.applicationData()), request.getData()); - ByteSizeValue actualSize = ByteSizeValue.ofBytes(serializationSize(labels) + serializationSize(data)); + final int size; + try (XContentBuilder builder = XContentFactory.jsonBuilder()) { + builder.startObject(); + builder.field("labels", labels); + builder.field("application_data", data); + builder.endObject(); + size = BytesReference.bytes(builder).length(); + } catch (IOException e) { + // This should not happen with in-memory builders + throw new ElasticsearchException("failed to calculate profile size", e); + } + final ByteSizeValue actualSize = ByteSizeValue.ofBytes(size); if (actualSize.compareTo(limit) > 0) { throw new ElasticsearchStatusException( Strings.format( "cannot update profile [%s] because the combined profile size of [%s] exceeds the maximum of [%s]", request.getUid(), actualSize, limit ), RestStatus.BAD_REQUEST ); } } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the profile size calculation is inaccurate because it omits the overhead of the JSON field names and structure, which could lead to incorrect quota enforcement.	Medium
More