[FEATURE] Move alerting from stashContext to the plugin subject #2318

Background

Most plugins in the distribution are moving their own system index work from ThreadContext.stashContext() to IdentityAwarePlugin with a PluginClient, tracked in opensearch-project/opensearch-plugins#238 under the RFC opensearch-project/security#4439. A stashed context carries no user, so the security plugin lets the request through without evaluating it. A request made through the plugin subject is evaluated, and is allowed on the plugin's own registered system indices and on whatever its plugin-additional-permissions.yml declares. common-utils now has a shared abstract client for this, opensearch-project/common-utils#1023.

alerting already implements IdentityAwarePlugin and has its own PluginClient in org.opensearch.alerting.util, which the resource sharing search paths of get destinations and search monitors use. main still has 43 stashContext() calls. This issue proposes how to move them, and asks for a decision on one part that does not have an obvious answer: indices named by a monitor's data_sources.

What the 43 calls do

Grouped by what the stashed block touches (file and line numbers are at f9b2214):

• Alerting's own registered indices (.opendistro-alerting-config, .opendistro-alerting-alert*, .opensearch-alerting-finding*, .opensearch-alerting-comments*): about 30 calls, including the four *Stashed helpers in SdkClientExtensions.kt. These can move to the plugin client.
• Indices named by monitor.dataSources when they are not the defaults, or by alertIndex and findingIndex on a transport request: reached from 15 of the calls (acknowledge, get alerts, get findings, index monitor and workflow, execute monitor and workflow, and the alert write in AlertService). See below.
• Another plugin's index read for the monitor owner: the anomaly detection result search in InputService.kt:203 and the detector lookup in TransportIndexMonitorAction.kt:605. The alerting plugin subject is not allowed on another plugin's system indices, so these stay as they are until anomaly detection offers an action that alerting can call as the owner.
• Calls that clear the user slot before roles or user info are injected for a monitor owner: MonitorRunner.kt:65, WorkflowRunner.kt:55, TransportDocLevelMonitorFanOutAction.kt:703, and the two in destinationmigration. These stay. The injection only fills an empty slot, so the stash is required.
• Context handling only: the two remote cluster paths (InputService.kt:480, TransportGetRemoteIndexesAction.kt:119), the job poller's clean context (MonitorJobPoller.kt:172), and four blocks that stash and then restore the caller's context for a resource sharing lookup. The last four can simply be removed once the surrounding code runs on the plugin client.

Two things about the current code matter for the change:

1. In about 15 of the calls the stash wraps scope.launch (or runner.launch). Coroutines do not carry the thread context and the stash is closed as soon as launch returns, so the body runs with whatever context the dispatcher thread holds. Replacing the stash line alone does not change what the body runs as. Each body has to use the plugin client explicitly, and needs the caller's context carried in, so that the resource sharing record still names the creator.
2. Scheduled runs dispatch ExecuteMonitorAction and ExecuteWorkflowAction (MonitorRunnerService.kt:373,413), so the stashes in TransportExecuteMonitorAction cover every scheduled run as well as the API.

Indices that are not registered

getSystemIndexDescriptors() covers the four patterns above. The findings pattern was added in #2310. Two indices alerting creates and owns are not covered:

• .opensearch-alerting-queries*, the default doc-level query index (DocLevelMonitorQueries.kt:93-94), and its rollovers.
• .opensearch-alerting-config-lock, the job lock index (LockService.kt:41).

Proposed:

SystemIndexDescriptor("${ScheduledJob.DOC_LEVEL_QUERIES_INDEX}*", "Alerting doc-level query index pattern"),
SystemIndexDescriptor(LockService.LOCK_INDEX_NAME, "Alerting job lock index"),

Neither pattern overlaps another registered pattern. Existing indices become system indices on restart with no data change. Secured tests like the ones in #2310 would cover direct access.

Data source indices

DataSources lets a caller name the query, findings, alerts and alert history indices and their patterns. The model checks only that three of them are not empty. The REST layer rejects custom values for query_index, findings_index and alerts_index, so in practice custom names come from transport callers. In the distribution that is security-analytics, which names them .opensearch-sap-<log type>-detectors-queries, -alerts, -alerts-history, -findings and so on (DetectorMonitorConfig.java). Those are ordinary indices; security-analytics does not register them.

A static descriptor cannot cover them, and two facts in core limit the choices:

• Descriptors are registered once at node start; there is no API to add one later.
• Core refuses to start a node if two descriptors overlap. A pattern such as .opensearch-sap-*-alerts* in alerting would overlap security-analytics' own .opensearch-sap-correlation-alerts.

Options considered:

1. Require custom names under a new alerting namespace such as .opensearch-alerting-ds-*, registered by alerting. Uses existing APIs, but security-analytics would have to rename its indices, existing detectors would need a migration, and anyone reading those indices directly would lose access once they become system indices.
2. Register patterns in alerting that cover the security-analytics names. Blocked by the overlap check.
3. Keep a stash for custom names only, in one helper, and use the plugin client for the defaults. No user visible change, but the custom path stays unevaluated.
4. Run custom name operations as the caller, or as the stored owner with roles injected. Monitors that security-analytics creates are stored with an empty user, because it calls alerting from a stashed context, so they would have no roles and fail.
5. Grant the security-analytics patterns to the alerting plugin subject in a new plugin-additional-permissions.yml, and validate at the transport layer that custom names match either alerting's registered patterns or a granted pattern. Uses existing APIs and changes nothing for current users. It ties alerting to security-analytics' naming, and if security-analytics later registers those indices as its own system indices, the grant no longer applies.
6. A delegated or runtime grant in the security plugin, where one plugin lets another act on some of its patterns. Needs a new security API.
7. Runtime descriptor registration. Needs a new core API, and system index status would have to be the same on every node.

The proposal is option 5, with option 3 as a bridge: stored monitors whose names match neither set keep working through the helper with a deprecation warning, and are rejected from the next major. The grant would be close to:

cluster_permissions:
  - "cluster:monitor/health"
  - "indices:data/read/mget"
  - "indices:data/read/msearch"
  - "indices:data/read/scroll"
  - "indices:data/read/scroll/clear"
index_permissions:
  - index_patterns:
      - ".opensearch-sap-*-detectors-queries*"
      - ".opensearch-sap-*-alerts*"
      - ".opensearch-sap-*-findings*"
    allowed_actions:
      - "indices:admin/create"
      - "indices:admin/mapping/put"
      - "indices:admin/aliases*"
      - "indices:admin/rollover"
      - "indices:admin/delete"
      - "indices:data/read/search*"
      - "indices:data/read/get"
      - "indices:data/write/index*"
      - "indices:data/write/update*"
      - "indices:data/write/delete*"
      - "indices:data/write/bulk*"

Action group names in this file do not resolve on released versions of the security plugin, so the list uses literal names. Writes to an index that is not a registered system index are evaluated as indices:data/write/bulk[s] on the shard, hence the bulk entry. The exact action list needs a run on a secured cluster to confirm. The cluster permissions are needed on every path, not only for data sources: mget, scroll (delete by query) and cluster health are used inside blocks that move to the plugin client.

Option 1 can be added later for new callers without conflicting with this.

Behaviour changes to expect on the plugin client

• With monitor or alerting-workflow protected by resource sharing, a search the plugin subject runs on .opendistro-alerting-config is filtered to the caller's principals. That is what list endpoints want. It is not what the workflow delegate check in DeleteMonitorService.monitorIsWorkflowDelegate wants, since that check is meant to see every workflow. Documents on that index that are not resources (monitor and workflow metadata, legacy destinations, email accounts and email groups) have no all_shared_principals, so a filtered search does not return them to a user who is not an admin. The resource sharing search for destinations already runs on the plugin client today.
• GETs by id on a system index are not realtime under the plugin subject until Keep plugin reads of their own system index realtime security#6668 is released. Paths that read their own write in the same request (monitor create then metadata read, execute of a monitor just written, get right after create) need to be listed in the PR.
• The plugin subject's own system index access applies when plugins.security.system_indices.enabled is true. Secured tests should run with it on.

Plan

1. Register the two indices.
2. Replace org.opensearch.alerting.util.PluginClient with a final subclass of the common-utils PluginClient, and build the system index SdkClient on it. Remove the *Stashed helpers.
3. Route every data source operation through one helper: plugin client for alerting's own names, the bridge for others.
4. Move the own-index calls and the coroutine bodies to the plugin client, and remove the caller restore blocks.
5. Add plugin-additional-permissions.yml with the cluster permissions, then the data source grant and the transport validation.
6. Add secured tests with a user who is not an admin, with resource sharing on and off.

Questions for maintainers:

• Is naming security-analytics' index patterns in alerting acceptable (option 5), or would you prefer option 1 with a migration in security-analytics?
• Should the workflow delegate check stay global? If so it needs a way to search alerting's own index without the resource sharing filter, which the security plugin does not offer today.
• Are there transport callers other than security-analytics that set custom data sources?

Related: opensearch-project/opensearch-plugins#238, opensearch-project/security#4439