Skip to content

Parallel plugin branches lose each other's payload mutations #158

Description

@araujof

Problem

CmfPluginInvoker::invoke is a read-modify-write across an await: it snapshots the shared payload under the lock, releases it, awaits invoke_entries, then writes the plugin's returned payload back wholesale.

A route with parallel: ["plugin(redact-ssn)", "plugin(scrub-token)"] spawns both branches onto a JoinSet sharing one CmfPluginInvoker. Both snapshot the same original payload, each plugin redacts its own field, and the second write overwrites the first. One redaction is lost.

The read-modify-write predates #157. What #157 changes is the consequence: the sticky payload_modified flag now makes the route handler forward the surviving payload and assert it was modified, so a lost redaction is forwarded to the host as a successfully-mutated payload rather than being uniformly dropped.

Separately, apl_core::rules::Effect::Plugin returns false from contains_mutation, so validate_parallel_purity permits plugin(...) inside parallel: in the first place. apl_cpex::parallel_safety::validate_parallel_plugin_modes exists and is unit-tested, but is never called during route registration — so nothing rejects a payload-modifying plugin under parallel: at config load.

Two candidate fixes (choosing between them is the point of this issue)

  1. Reject at load. Wire validate_parallel_plugin_modes into the route-registration path in visitor.rs alongside the other post-IR validations, so a Sequential/Transform plugin inside parallel: fails config load. Smaller, and matches how FieldOp and Delegate are already refused inside parallel:. Cost: narrows what operators can express.
  2. Close the window. Hold a single MutexGuard on self.payload across invoke_entries, or re-read under the lock and merge the returned payload the way apply_changed_paths merges pipeline edits, rather than assigning wholesale. Cost: serializing branches through the payload mutex partly defeats the purpose of parallel:.

Reviewers split between these, which is why this is filed rather than fixed in #157.

Evidence

  • crates/apl-cpex/src/cmf_invoker.rs:335let current_payload = self.payload.lock().await.clone(); (snapshot, lock released at end of statement)
  • crates/apl-cpex/src/cmf_invoker.rs:381*self.payload.lock().await = modified.clone(); (wholesale write-back of whatever a sibling branch wrote in between)
  • crates/apl-core/src/rules.rs:282-291Effect::Plugin returns false from contains_mutation, so validate_parallel_purity allows it inside parallel:
  • crates/apl-cpex/src/parallel_safety.rsvalidate_parallel_plugin_modes exists and is tested, but has no caller in the registration path
  • crates/apl-core/src/evaluator.rs:1006-1028dispatch_parallel spawns branches with Arc::clone(plugins), so they share one invoker

Suggested test

A parallel: block with two mutating plugins either fails to load (fix 1) or preserves both mutations (fix 2). Neither direction is covered today.


Severity: P2
Confidence: 100 (merged from three independent reviewers at 75/50/50)
Reviewer(s): adversarial, correctness, security
Finding ID: crates/apl-cpex/src/cmf_invoker.rs:381:parallel-branches-lose-payload-mutations
Surfaced by: code review of #157
Run artifact: /tmp/compound-engineering/ce-code-review/20260806-022016-2517f78a/

Metadata

Metadata

Assignees

Labels

Type

Projects

Status
Backlog

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions