Hide metadata field from REST API responses using secure flag

[riysaxen-amzn]

Move metadata serialization inside the !secure block in Monitor.createXContentBuilder() so it is excluded from customer-facing REST responses (toXContent with secure=true) while still included in internal paths like index persistence and SQS payloads (toXContentWithUser with secure=false). This follows the same pattern used for the user field.

Description

[Describe what this change achieves]

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

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.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 3b01abf)

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 Test Coverage Gap The test helper serializeMonitor was changed to use toXContentWithUser (secure=false), but there is no corresponding test verifying that toXContent (secure=true) actually excludes the metadata field. The PR's core claim — that metadata is hidden from REST API responses — is not validated by any test. monitor.toXContentWithUser(builder, ToXContent.EMPTY_PARAMS) Behavioral Change Moving metadata inside the !secure block means it will now be absent from all REST API responses where secure=true. If any existing consumers (e.g., GET Monitor API clients) rely on reading metadata from the REST response, this is a breaking change that should be explicitly documented and validated. if (!secure) { builder.optionalUserField(USER_FIELD, user) if (!metadata.isNullOrEmpty()) builder.field(METADATA_FIELD, metadata) }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 3b01abf
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Guard against data loss in storage serialization The metadata field is now hidden when secure=true, but this also means metadata will be absent when the monitor is persisted to the index (if the same serialization path is used for storage). Confirm that the secure flag is only true for REST API responses and not for index storage serialization, otherwise metadata could be lost on read-modify-write cycles. src/main/kotlin/org/opensearch/commons/alerting/model/Monitor.kt [190-193] if (!secure) { builder.optionalUserField(USER_FIELD, user) if (!metadata.isNullOrEmpty()) builder.field(METADATA_FIELD, metadata) +} else { + // metadata intentionally excluded from REST API response when secure=true + // ensure this path is NOT used for index persistence } Suggestion importance[1-10]: 5 __ Why: This raises a valid concern about potential data loss if the secure=true path is used for index persistence, as metadata would be omitted. However, the improved_code only adds a comment rather than implementing an actual fix, and the suggestion asks to "confirm" behavior rather than providing a concrete solution.	Low
General	Verify test coverage for secure flag behavior The serializeMonitor function is used in scheduled job payload tests, which likely need to serialize the full monitor including user and metadata fields. However, toXContentWithUser may behave differently than toXContent with respect to the secure flag, potentially causing tests to not cover the metadata hiding behavior introduced in this PR. Verify that the test cases explicitly assert that metadata is hidden when secure=true and visible when secure=false. src/test/kotlin/org/opensearch/commons/alerting/model/ScheduleJobPayloadTests.kt [19] monitor.toXContentWithUser(builder, ToXContent.EMPTY_PARAMS) +// Ensure tests cover both secure=true (metadata hidden) and secure=false (metadata visible) scenarios Suggestion importance[1-10]: 3 __ Why: The suggestion asks to verify test coverage rather than proposing a concrete code fix. The improved_code is essentially the same as existing_code with just a comment added, making this a low-impact suggestion.	Low

---

Previous suggestions

Suggestions up to commit d4e7209

Category	Suggestion	Impact
Possible issue	Verify metadata exclusion doesn't affect index storage The metadata field may contain sensitive information and is now correctly hidden when secure is true. However, consider verifying that the metadata field does not need to be persisted to the index separately from the REST API response, since moving it inside the secure block could affect index serialization if the same toXContent method is used for both REST responses and index storage. src/main/kotlin/org/opensearch/commons/alerting/model/Monitor.kt [190-193] if (!secure) { builder.optionalUserField(USER_FIELD, user) if (!metadata.isNullOrEmpty()) builder.field(METADATA_FIELD, metadata) +} else { + // metadata is intentionally excluded from REST API responses when secure=true + // Ensure index serialization path handles metadata separately if needed } Suggestion importance[1-10]: 4 __ Why: The suggestion raises a valid concern about whether moving metadata inside the secure block affects index serialization, but the improved_code is essentially the same as existing_code with just a comment added, making it more of a verification request than an actionable fix. The concern itself is worth noting but the suggestion doesn't provide a concrete solution.	Low

[github-actions]

Persistent review updated to latest commit 3b01abf