Skip to content

Prevent malformed retry delays from poisoning Nexus completions - #11617

Merged
Quinn-With-Two-Ns merged 1 commit into
mainfrom
NEXUS-627
Aug 19, 2026
Merged

Prevent malformed retry delays from poisoning Nexus completions#11617
Quinn-With-Two-Ns merged 1 commit into
mainfrom
NEXUS-627

Conversation

@Quinn-With-Two-Ns

@Quinn-With-Two-Ns Quinn-With-Two-Ns commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

What changed?

Validate the retry delay is a valid proto duration.

Why?

Prevent malformed retry delays from poisoning Nexus completions.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

Potential risks

Potentially users could have been sending an invalid proto duration, unclear how exactly, and now we would fail their request.


Note

Medium Risk
Request validation on the activity-failure path may reject clients that previously sent invalid duration payloads; behavior change is intentional but could surface latent bad callers.

Overview
RespondActivityTaskFailed and RespondActivityTaskFailedById now reject failures whose ApplicationFailureInfo.NextRetryDelay is not a valid google.protobuf.Duration, including nested causes in the failure chain.

A new validateActivityFailureNextRetryDelays helper runs CheckValid() on any set delay and returns InvalidArgument with a clear message instead of accepting malformed values that could break downstream handling (e.g. Nexus completions).

Unit tests cover edge cases (range, sign mismatch, nested invalid delay); functional tests confirm both respond-by-token and respond-by-ID paths leave the activity RUNNING when validation fails and still allow a valid failure afterward.

Reviewed by Cursor Bugbot for commit b7bb95b. Bugbot is set up for automated code reviews on this repo. Configure here.

@Quinn-With-Two-Ns
Quinn-With-Two-Ns requested a review from a team August 18, 2026 18:15
@Quinn-With-Two-Ns
Quinn-With-Two-Ns requested review from a team as code owners August 18, 2026 18:15
@Quinn-With-Two-Ns

Copy link
Copy Markdown
Contributor Author

For the record the bug report from Codex is as follow

ApplicationFailureInfo.NextRetryDelay is a google.protobuf.Duration. Binary protobuf and gRPC can carry a structurally invalid duration, such as {seconds: 1, nanos: -1}, but protobuf JSON rejects it. RespondActivityTaskFailed validates the failure variant and byte size without validating this nested duration, and the standalone activity reads it with AsDuration without checking CheckValid. If the failure is terminal, the malformed duration remains in the persisted failure.

Nexus completion later converts the entire Temporal failure with protojson.Marshal. That conversion rejects the duration before either the internal or system callback invoker is constructed, so the target Nexus operation never receives the already-committed activity outcome. The callback task returns an internal processing error and remains scheduled for queue retries, or eventually reaches the DLQ when that policy is enabled; retrying cannot repair the persisted failure.

Evidence:

Impact:

  1. A buggy or malicious worker reports a terminal standalone activity failure with a small but invalid NextRetryDelay duration.
  2. The frontend accepts the binary gRPC request, and History closes the activity with that failure.
  3. The activity's Nexus callback cannot serialize its completion arguments, so no request reaches the target namespace.
  4. The backing Nexus operation remains running until its own timeout, or indefinitely if it has none, while the source callback repeatedly fails or is sent to the DLQ.

@Quinn-With-Two-Ns Quinn-With-Two-Ns added the reliability-2026 Reliability related changes label Aug 19, 2026
@Quinn-With-Two-Ns
Quinn-With-Two-Ns merged commit 7fc5c02 into main Aug 19, 2026
55 checks passed
@Quinn-With-Two-Ns
Quinn-With-Two-Ns deleted the NEXUS-627 branch August 19, 2026 21:09
davidporter-id-au pushed a commit to davidporter-id-au/temporal that referenced this pull request Aug 24, 2026
…oralio#11617)

## What changed?
Validate the retry delay is a valid proto duration.

## Why?
Prevent malformed retry delays from poisoning Nexus completions.

## How did you test it?
- [ ] built
- [ ] run locally and tested manually
- [ ] covered by existing tests
- [x] added new unit test(s)
- [x] added new functional test(s)

## Potential risks
Potentially users could have been sending an invalid proto duration,
unclear how exactly, and now we would fail their request.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reliability-2026 Reliability related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants