Uh oh!
There was an error while loading. Please reload this page.
Backport #2035: [world-vercel] Validate ref resolve responses before use - #2297
Merged
Conversation
* [world-vercel] Validate ref resolve responses before use When workflow-server returns a ref body to the SDK, the bytes are fed into the workflow runtime's event log and deserialized via `decodeFormatPrefix`. The SDK always writes ref payloads with at least a 4-byte format prefix (see `encodeWithFormatPrefix` in `@workflow/core`), so a zero-byte response — or one whose length disagrees with `Content-Length` — is never a valid stored value. Before this change, `resolveRefDescriptor` had no validation: a 200 with an empty body would be passed downstream as a zero-length Uint8Array, which then failed deep inside replay with: Data too short to contain format prefix: expected at least 4 bytes, got 0 By that point the workflow's in-memory event snapshot is already poisoned with the empty payload, so every subsequent replay deterministically reproduces the same failure, downstream `resumeHook()` calls surface as `Hook not found`, and the run only unsticks when stale-run cleanup terminates the sandbox. This catches the failure at the transport boundary instead, where it can be retried as a `WorkflowWorldError`. Both an empty body and a length mismatch (truncated streaming response) are rejected. This is the SDK-side companion to vercel/workflow-server#432, which adds the same validation on the server side. * Address review: reject <4-byte bodies, handle malformed Content-Length Three review changes: 1. Reject any body shorter than the 4-byte format-prefix length, not just zero-byte bodies. The SDK guarantees every stored ref payload starts with a 4-byte format prefix (FORMAT_PREFIX_LENGTH in @workflow/core), so a 1-3 byte body would also fail downstream replay with the same 'Data too short to contain format prefix' error this PR exists to prevent. 2. Parse Content-Length safely with parseInt + Number.isFinite + non-negative checks instead of bare Number(). A non-numeric value like 'abc' would otherwise produce NaN and silently surface as a 'truncated' error, masking the real cause. Malformed values are treated as absent; the minimum-length check still defends against actual truncation in that case. 3. Add tests for the truncated-body-without-Content-Length case (chunked transfer where Content-Length validation can't see the truncation), and for a malformed Content-Length header that should be ignored rather than misreported as truncation. The validation logic also moves into a small assertValidRefBody helper to keep the inner trace function under the noExcessiveCognitiveComplexity limit. * Address review: scope 4-byte minimum to binary refs, strict Content-Length parsing - Only apply the 4-byte format-prefix minimum to application/octet-stream payloads; CBOR refs can legitimately be 1-byte primitives (true/0/null). - Require Content-Length to be a plain run of digits before comparing; parseInt would otherwise accept numeric-prefixed garbage ('12junk' -> 12). - Make the changeset succinct. * Address review: skip Content-Length check for compressed responses fetch/undici transparently decompresses gzip/br bodies but leaves Content-Length describing the encoded (compressed) size, so comparing it against the decompressed byteLength would reject valid compressed refs as a phantom 'ref-body-length-mismatch'. Skip the comparison when a non-identity Content-Encoding is present; an absent or 'identity' encoding is still validated. Adds regression tests for both cases. Signed-off-by: Nathan Rajlich <n@n8.io>
🦋 Changeset detectedLatest commit: fc9c6de The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated backport of #2035 to
stable(backport job run).AI recommendation: This is a self-contained bug fix to
resolveRefDescriptorin@workflow/world-vercel, which exists onstablewith the same pre-change content (blobbae3ae6a). The change adds defense-in-depth validation against a production failure mode (empty/truncated ref bodies corrupting event-log replay) and is independent of any main-only APIs.