feat(core): seal hook payloads to the target run's public key - #3096
Conversation
🦋 Changeset detectedLatest commit: 937917b 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
This PR updates @workflow/core to prefer sealing resumeHook() payloads to the target run’s published X25519 public key (when present), eliminating the cross-deployment run-key lookup while widening all relevant read sites to resolve the full payload-key capability so sealed payloads can be opened.
Changes:
resumeHook()now seals toWorkflowRun.encryptionPublicKeywhen available, falling back to the symmetric-key path when absent/malformed or when encryption is off.- Reader-side key resolution is widened from
CryptoKeytoPayloadKey/RunPayloadKeysso runs can open bothencrandencppayloads. - Adds/updates tests to cover sealing, fallbacks, and end-to-end openability of sealed payloads.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/workflow.ts | Widen runWorkflow encryption key parameter from CryptoKey to PayloadKey. |
| packages/core/src/step/context-storage.ts | Widen step context encryptionKey type to PayloadKey. |
| packages/core/src/serialization/encryption.ts | Add deriveRunPayloadKeys() to resolve full run capability (AES + keypair). |
| packages/core/src/serialization.ts | Re-export sealed-box key variants/utilities from the public serialization surface. |
| packages/core/src/sealed-box.ts | Document AAD rationale and add decodeRunPublicKey() helper for safe public-key parsing. |
| packages/core/src/runtime/step-executor.ts | Widen step executor encryptionKey type to PayloadKey. |
| packages/core/src/runtime/runs.ts | Use deriveRunPayloadKeys() when recreating runs so sealed payloads can be opened. |
| packages/core/src/runtime/run.ts | Widen cached run key type and resolve via deriveRunPayloadKeys(). |
| packages/core/src/runtime/resume-hook.ts | Prefer sealing to published run public key; retain symmetric fallback + capability checks for encr. |
| packages/core/src/runtime/resume-hook.test.ts | Add sealing/fallback tests and an end-to-end “seal then hydrate” assertion. |
| packages/core/src/runtime/helpers.ts | Memoized per-run key accessor now resolves PayloadKey via deriveRunPayloadKeys(). |
| packages/core/src/runtime/helpers.test.ts | Add tests ensuring memoized keys can open both sealed and symmetric payloads and are cached. |
| packages/core/src/replay-payload-cache.ts | Widen replay payload cache constructor key type to PayloadKey. |
| packages/core/src/private.ts | Widen orchestrator context encryptionKey type to PayloadKey. |
| .changeset/encp-resume-hook-seal.md | Changeset documenting sealing behavior and widened reader key capability. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
881ce18 to
34056d5
Compare
8ba434a to
136f3a3
Compare
📊 Workflow Benchmarkscommit Backend:
📜 Previous results (3)54bab0eTue, 28 Jul 2026 00:06:34 GMT · run logs
54bab0eMon, 27 Jul 2026 22:14:33 GMT · run logs
8161db1Mon, 27 Jul 2026 20:08:39 GMT · run logs
ℹ️ 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 |
karthikscale3
left a comment
There was a problem hiding this comment.
ai review: The rebased head is otherwise clean and all checks pass, but the existing review finding remains blocking for this PR as an independently mergeable change: it begins emitting sealed (encp) hook payloads while workflow inspect --decrypt still constructs only a symmetric CryptoKey, so those payloads surface as ciphertext placeholders. The implementation fix exists in stacked PR #3097; please either land that fix with this change or ensure the two cannot merge/release separately.
|
@karthikscale3 you're right, and the ordering is now fixed: the o11y decrypt work lands before this PR. It is in #3146 (base To make the reader side stand alone, After #3146 merges, this PR's rebase will hit one expected conflict: it currently also adds One caveat worth flagging: #3097 is showing as "merged" in the UI but its code is not in |
This is the payoff for the sealed-box work: cross-deployment `resumeHook()` no longer calls `getEncryptionKeyForRun`, which on Vercel means a ~350ms `run-key` API round trip (Cosmos reads, a 50–75KB `encrypted_env.json` fetch from S3, up to three KMS decrypts) to recover 32 bytes. When the target run publishes a public key, the resumer seals to it using only the run entity it already fetched. More than half of `resumeHook()` calls miss the same-deployment fast path, so this is the dominant term in hook-resumption latency — and for Eve, hook resumption is what an agent turn waits on. It also reduces privilege. Fetching the symmetric key grants read access to everything in the run; sealing grants only the ability to write one payload to it. A resumer is now cryptographically unable to read the run it resumes. Sealing is gated on the presence of `encryptionPublicKey`, deliberately *not* on the capability version table the way `encr` and `gzip` are. A run only carries a public key if the runtime that created it could also open a sealed payload, and runs are pinned to their creating deployment — so presence is a stronger attestation than comparing versions, and it stays correct when `@workflow/core` and `@workflow/world-vercel` versions drift independently (each single-axis version gate wedges a run under one drift direction; presence wedges under neither). `resumeWebhook` keeps using the symmetric key it already had to fetch in order to hydrate hook metadata — sealing there would add an ECDH without saving a round trip. **Read path.** Sealing is useless if the run cannot open the result, and every reader previously resolved a bare symmetric `CryptoKey`, which by construction cannot open `encp`. Key resolution now yields the full capability (symmetric key + X25519 keypair) at every read site: `memoizeEncryptionKey`, the `Run` class, `runs.ts`, and `getHookByTokenWithKey`. Without this the first sealed hook payload would have wedged its run with `RuntimeDecryptionError`. Holder types widen from `CryptoKey` to `PayloadKey`, which is a pure widening. Falls back to the symmetric path when the run has no public key (older SDKs), when the stored value is malformed, or when encryption is off — all covered by tests, along with an end-to-end assertion that a sealed payload actually hydrates with the keys the owning deployment re-derives.
- `memoizeEncryptionKey`'s JSDoc still described importing an AES-256 `CryptoKey`. It now resolves a run's full capability (symmetric key plus X25519 keypair), and the reason matters: a run reading its own event log can meet sealed payloads another run wrote to it, and resolving only the symmetric key would leave those unopenable. Documented explicitly so the next reader does not "simplify" it back. - A comment in `resumeHook` pointed at the `encr` capability gate as being "above" when it is in the fallback branch below. Reworded. - Added the `decodeRunPublicKey` boundary test that belongs with the strict base64 work but needed this branch's code to exist.
|
@karthikscale3 ready for re-review — the ordering concern from your review is resolved. #3146 (the o11y decrypt work) is merged, so This branch is rebased onto that main ( Post-rebase verification: build clean, core 1591, world-vercel 299, cli 78 passing. Changeset re-checked too — this PR touches only One heads-up unrelated to this PR: #3097 shows as "merged" in the UI but its code never reached main. I force-pushed its base branch to a commit that contained its head, and GitHub auto-closed it as merged. #3146 is the real landing of that work. |
|
No backport to This is feature work: PR 4 of a 7-PR stack that introduces sealed-box ( To override, re-run the Backport to stable workflow manually via |
PR 4 of 7 — the payoff. Stacked on #3095; 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 awaitsImpact
Cross-deployment
resumeHook()no longer callsgetEncryptionKeyForRun, which on Vercel means a ~350msrun-keyAPI round trip — Cosmos reads, a 50–75KBencrypted_env.jsonfetch from S3, up to three KMS decrypts — to recover 32 bytes. When the target run publishes a public key, the resumer seals to it using only the run entity it already fetched.More than half of
resumeHook()calls miss the same-deployment fast path, so this is the dominant term in hook-resumption latency. For Eve, hook resumption is what an agent turn waits on.Also removes the sfo1 single-region dependency, the S3/KMS load, and the 2000 req/min/owner rate-limit exposure from the resumption hot path.
It also reduces privilege
Fetching the symmetric key grants read access to everything in the run. Sealing grants only the ability to write one payload to it. A resumer is now cryptographically unable to read the run it resumes — upgrading the previous honor-system
importKey(raw, ['encrypt'])restriction to a real guarantee.Sealing is useless if the run can't open the result — and every reader previously resolved a bare symmetric
CryptoKey, which by construction cannot openencp. Shipping the seal path alone would have wedged the first sealed hook payload withRuntimeDecryptionError.Key resolution now yields the full capability (symmetric key + X25519 keypair) at all four read sites:
memoizeEncryptionKey, theRunclass,runs.ts, andgetHookByTokenWithKey. Holder types widen fromCryptoKeytoPayloadKey— a pure widening. This is most of the diff, and it's why this PR is bigger than "make resumeHook seal".Gating
Sealing is gated on presence of
encryptionPublicKey, deliberately not on the capability version table the wayencrandgzipare — see #3095 for why presence is the stronger attestation.resumeWebhookkeeps using the symmetric key it already had to fetch (to hydrate hook metadata); sealing there would add an ECDH without saving a round trip.Falls back to the symmetric path when the run has no public key (older SDKs), when the stored value is malformed, or when encryption is off.
🔍 Open decision: no AAD on sealed payloads
An earlier iteration of this design specified additional authenticated data of
projectId|runIdon every sealed payload. This PR seals with no AAD, because:ephemeralPublicKey ‖ recipientPublicKey, and a recipient public key is unique per (deployment key × project × run). Replaying a sealed payload at a different run therefore already fails at key agreement.The
aadparameter remains available in the sealed-box API for callers that want a KDF-independent binding, and the reasoning is documented insealed-box.ts. Flagging it explicitly because it reverses an earlier design decision — it should be a review call, not mine.Tests
8 tests on
resumeHookcovering: no key fetch when a public key is present, an end-to-end assertion that the sealed payload hydrates with the keys the owning deployment re-derives, all three fallback paths, presence-gating overriding an old reported core version, and the webhook override path. Plus 4 onmemoizeEncryptionKeyasserting the resolved key opens both schemes.Full core suite (1596) passes; workspace typecheck clean; zero new lint diagnostics (verified against a stashed baseline).