feat(core): route sealed envelopes through the serialization layer - #3094
Conversation
🦋 Changeset detectedLatest commit: c748b63 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 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 |
🧪 E2E Test Results✅ All tests passed E2E Test SummarySummary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
✅ vercel-multi-region
|
There was a problem hiding this comment.
Pull request overview
Routes sealed-box (encp) envelopes through the core serialization encryption layer by widening the encryption-key surface area from a symmetric-only CryptoKey to a PayloadKey union that can represent either symmetric encryption (encr) or asymmetric sealing targets (encp). This enables cross-run writers to encrypt using only a recipient public key while keeping same-run payloads symmetric for efficiency.
Changes:
- Introduce
PayloadKey = CryptoKey | SealTarget | RunPayloadKeysand implementencpsealing/opening inserialization/encryption.ts. - Update one-shot and streaming serialization paths to accept
PayloadKeyand correctly read/writeencpframes/envelopes. - Add unit tests covering sealed envelope behavior, capability enforcement, and stream framing properties for
encp.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/serialization/encryption.ts | Adds PayloadKey union + SealTarget/RunPayloadKeys variants and encp encrypt/decrypt support alongside existing encr. |
| packages/core/src/serialization.ts | Wires PayloadKey through stream and helper APIs; stream decrypt now supports encp frames with clearer error reporting. |
| packages/core/src/serialization/step.ts | Widen step-mode serialize/deserialize encryption key param to PayloadKey. |
| packages/core/src/serialization/client.ts | Widen client-mode serialize/deserialize encryption key param to PayloadKey. |
| packages/core/src/serialization/serialization.test.ts | Adds tests for encp envelopes, runtime type guards, and capability enforcement. |
| packages/core/src/serialization.test.ts | Adds end-to-end stream sealing (encp) framing/round-trip/tamper and regression tests. |
| .changeset/encp-payload-key-union.md | Declares the @workflow/core minor bump and summarizes the new PayloadKey behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8a44694 to
2712e15
Compare
karthikscale3
left a comment
There was a problem hiding this comment.
ai review: Reviewed the PayloadKey routing and sealed stream integration on the current head. The key variants preserve existing symmetric behavior while preventing X25519 public keys from reaching AES-GCM by mistake. The stream sessions correctly amortize one KEM per writer, retain fresh random nonces per frame, bound the read cache, and handle interleaved writers and failed decapsulation. The sole existing inline performance finding is fixed and resolved. The broad Vercel production E2E failures match the unrelated matrix-wide failures on the stack; focused unit, module-boundary, local framework, Windows, and multi-region checks pass.
Adds the plumbing that lets a cross-run writer emit `encp` payloads. The serialization layer previously had no way to express "seal to this public key": every consumer expected a symmetric `CryptoKey`, and the encrypt primitive was unconditionally AES-GCM. That gap was also a live footgun. A 32-byte X25519 public key is a structurally valid AES-256 key, so `importKey(pubkey, 'AES-GCM')` succeeds and silently produces ciphertext nobody can ever open — no compile error, no runtime error, just unreadable data. Making public keys reachable only through `sealTo()` turns that mistake from "avoided by convention" into "unrepresentable". `PayloadKey` is a widening rather than a replacement, so no existing call site changes: | Variant | Writes | Reads | Held by | | ---------------- | ------ | ------------ | ----------------------- | | `CryptoKey` | encr | encr | same-run (legacy shape) | | `RunPayloadKeys` | encr | encr, encp | the owning run, o11y | | `SealTarget` | encp | — | cross-run writers | A run's own payloads deliberately stay symmetric even when the holder could seal: sealing costs a fresh ECDH and 32 bytes per envelope and buys nothing when the writer already holds the decryption key. The variants are branded with `Symbol.for` (not `Symbol()`) because these values cross the host ↔ workflow VM realm boundary, where only the global symbol registry is shared. Both stream directions handle sealed frames, keeping the length header in the clear so frame boundaries stay findable without a key. Frames keep per-frame random nonces rather than a counter — a reconnect or a durable replay restarts the writer, and a counter would repeat `(key, nonce)` and break AES-GCM catastrophically. Regression tests assert that 100 frames of identical plaintext produce 100 distinct ciphertexts, and that ten writer incarnations never share a content key. Capability shortfalls fail loudly and specifically: opening a sealed payload with a symmetric key (or a write-only seal target, or nothing) reports "no run keypair is available" rather than surfacing as a mysterious auth-tag failure further down.
Sealing each frame independently meant a fresh X25519 keygen + ECDH + HKDF per chunk — O(frames) crypto for a long stream. As review pointed out, amortizing the KEM is safe here so long as nonces stay random, which they already are; the hazard I had been guarding against was counter nonces specifically, not a long-lived content key. `createSealSession` performs one encapsulation per writer instance and reuses the content key, with a fresh random nonce per frame. `createOpenSession` mirrors it on the read side, caching decapsulation by ephemeral public key — since every frame from one writer carries the same key, that is an ECDH per writer rather than per frame. Both safety properties are preserved and now asserted directly: - nonces stay random, so sharing a content key cannot repeat `(key, nonce)` (100 identical-plaintext frames -> 100 distinct frames) - a session is scoped to one stream instance, so a reconnect or durable replay never inherits a previous content key (10 writer incarnations -> 10 distinct ephemeral keys) The envelope layout is byte-identical to one-shot `seal`, so readers cannot tell which path produced a frame and need no matching session. Two edge cases the read-side cache introduces, both covered: frames from two writers interleaved on one stream (eviction on every frame, so correctness cannot depend on hit rate), and a failed decapsulation clearing rather than poisoning the entry.
📊 Workflow Benchmarks❌ The benchmark run for Partial results from the failed run: commit Backend:
ℹ️ Metric definitions & methodologyBest/P75/P90/P99 deltas compare against the most recent benchmark run on Metrics — TTFS: time to first step body (in-deployment start() → first step body, deployment clocks) · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (whole-run time outside step bodies, in-deployment anchored) · SL: stream latency (in-deployment write → read propagation, readAt - writtenAt) · SO: stream overhead (end-to-end write+consume time beyond the modelled generation window) Scenarios — step: one trivial no-op step, no stream; no hooks, so the run stays in turbo mode (in-process fast path) · stream: one streaming step; no hooks, so the run stays in turbo mode (in-process fast path) · hook + stream: registers a hook before one step, which exits turbo mode (dispatch path) · 1020 steps: 1020 trivial sequential steps; STSO is measured between consecutive steps in the given step ranges, and WO is the whole-run overhead outside step bodies · stream latency: parallel reader/writer steps on a dedicated stream; SL is the in-deployment write->read propagation (readAt - writtenAt) · stream overhead (text): writer streams 300 variable-length text token deltas paced at 100/s for 3s (a haiku-size LLM's token throughput) while a parallel reader drains the whole stream; SO is the end-to-end write+consume time beyond the 3s generation window (overhead/backpressure) · stream overhead (structured): same workload as stream overhead (text), but each delta is an AI-SDK-style structured object ({ type: 'text-delta', id, text }) instead of a raw string, so the SO gap vs the text scenario is the added serialization cost 🔴 marks a percentile over its target (within target is left unmarked). Targets (p75/p90/p99, ms) — TTFS 200/300/600 · SL 50/60/125 · SO 250/500/1000 · STSO (1-20) 20/30/60 · STSO (101-120) 30/45/90 · STSO (1001-1020) 40/60/120 All metrics are measured from deployment-side timestamps only. Runs are triggered by an in-deployment route that stamps the anchor ( Cold starts are kept in the numbers on purpose — they are part of real bursty-workload latency. The workbench deployment cold-starts the |
|
No backport to This is feature work: PR 2 of a 7-PR stack that adds a new To override, re-run the Backport to stable workflow manually via |
PR 2 of 7. Stacked on #3093; review the stack in order, starting from #3093.
encpsealed-box encryption primitive #3093 — theencpsealed-box primitiveresumeHook()seals instead of fetching the key ← the payoffstart()gets the key from the probe it already awaitsThe gap this closes
The serialization layer had no way to express "seal to this public key": every consumer expected a symmetric
CryptoKey, and the encrypt primitive was unconditionally AES-GCM.That gap was also a live footgun. A 32-byte X25519 public key is a structurally valid AES-256 key, so
importKey(pubkey, 'AES-GCM')succeeds and silently produces ciphertext nobody can ever open — no compile error, no runtime error, just unreadable data. Making public keys reachable only throughsealTo()turns that from "avoided by convention" into "unrepresentable".PayloadKeyis a widening, not a replacementThis is why the diff touches ~6 files instead of ~60, and why no existing call site or test needed to change:
CryptoKeyencrencrRunPayloadKeysencrencr,encpSealTargetencpA run's own payloads deliberately stay symmetric even when the holder could seal: sealing costs a fresh ECDH and 32 bytes per envelope and buys nothing when the writer already holds the decryption key. The dual
encr/encpmodel is the intended steady state, not a migration step — theencrread path has to exist forever for already-written payloads regardless, and readers dispatch on the 4-byte format prefix, so converging on one scheme would delete no code.The variants are branded with
Symbol.forrather thanSymbol()because these values cross the host ↔ workflow VM realm boundary, where only the global symbol registry is shared.Streams
Both directions handle sealed frames, keeping the length header in the clear so frame boundaries stay findable without a key.
Frames keep per-frame random nonces rather than a counter. Regression tests assert:
Error quality
Capability shortfalls fail loudly and specifically. Opening a sealed payload with a symmetric key (or a write-only seal target, or nothing) reports "no run keypair is available" rather than surfacing as a mysterious auth-tag failure further down.
Design note: why no new
WorldmethodAn obvious alternative is to add a
World.getEncryptionPublicKeyForRun()accessor so callers can ask the world for a run's public key. This PR deliberately does not.The public key is data, not a capability: it arrives on the
WorkflowRunentity thatresumeHookalready fetches, and (in a later PR) in the serialized stream descriptor. Nothing needs to ask for it. Keeping it out of the interface means theWorldcontract is unchanged and customWorldimplementations get sealed-envelope support for free, with no new method to implement.Worth a second opinion on that call, since it is the main structural choice in this PR.
20 new tests. Full core unit suite (1576) passes; workspace typecheck clean.