feat: return the run public key from the capability probe#3099
feat: return the run public key from the capability probe#3099TooTallNate wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 5475251 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 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❌ Some tests failed ❌ Failed E2E Tests▲ Vercel Production (1 failed)hono (1 failed):
📦 Local Production (1 failed)nextjs-webpack-stable (1 failed):
E2E Test SummarySummary
Details by Category❌ ▲ Vercel Production
✅ 💻 Local Development
❌ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
✅ vercel-multi-region
|
0747839 to
ec659f4
Compare
8de4685 to
bf07d76
Compare
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
|
Found and fixed a bug in this PR during manual production verification — worth a careful look since it broke more than the new feature. SymptomA cross-deployment The target deployment's flow route logged: The probe was being executed as a workflow invoke. Cause
export const QueuePayloadSchema = z.union([
WorkflowInvokePayloadSchema, // { runId: string, ... } <-- matched first
StepInvokePayloadSchema,
HealthCheckPayloadSchema,
]);
The runtime dispatches on This is not limited to the new key feature. Any cross-deployment Fix (d16e185)
Note on the union orderingOrdering fixes the collision, but the union stays structurally ambiguous — any future optional field on |
Verified in production ✅Deployed d16e185 to two preview deployments and re-ran the case that previously failed 100% of the time. Probe with a
Distinct per-run keys, derived locally inside the target deployment. Cross-deployment So the workflow arguments are genuinely sealed with the sealed-box envelope rather than the Worth calling out: this path needs no key lookup at all. The probe supplies the key, so One follow-up concern, not addressed here
|
|
Note: I originally landed a fix on this branch for a bug that originates in #3095. It has since been moved down to #3095 where it belongs, so this PR is back to two commits. Full write-up is in #3095. Short version: With that fixed, the whole stack now works end to end in production, including the Two things reviewers should know about the restack:
|
This removes the last key-lookup request from the cross-deployment hot paths. `start()` already blocks on a capability probe for every cross-deployment call, and the probe responder executes *inside the target deployment*, where the run's key material is available locally. So the public key can ride back on a response the caller is already awaiting, at no additional latency, and the `run-key` API request disappears. Three properties make this better than keeping the request: - The wait is already being paid. Folding the key into the existing response removes the request outright rather than relocating it. - A public key is exactly what this channel can carry. The probe response stream is deliberately unauthenticated, which would disqualify shipping the symmetric key over it — but a public key is not secret. - It reduces privilege. The caller ends up able to seal the workflow arguments but not read them back; fetching the symmetric key granted full read access to a run it merely launched. `runId` is now minted before the probe rather than just after it. `createRunId()` reads only `opts`, which is fully resolved by that point, so the move has no other dependency — and a test asserts the id sent to the probe is the one actually created. Everything is best-effort. The probe is already failure-tolerant (2s timeout, errors swallowed) and is skipped entirely for same-deployment starts and for worlds without a streams API. When no key comes back — old target, timeout, encryption disabled, or a malformed value — `start()` falls back to the existing lookup plus symmetric encryption. Key derivation failures inside the responder are caught and logged so the probe still reports health and capabilities, which callers depend on for reasons unrelated to encryption.
`QueuePayloadSchema` is an ordered union and `z.object` strips keys the
matching member doesn't declare. Adding an optional `runId` to
`HealthCheckPayloadSchema` made a probe payload also satisfy
`WorkflowInvokePayloadSchema`, whose only required field is `runId`. Because
the invoke member came first, world-vercel's queue handler parsed a
runId-bearing probe down to `{ runId }`, dropping `__healthCheck` and
`correlationId`.
The runtime dispatches on `__healthCheck` before falling through to the
invoke schema, so the probe was reinterpreted as "replay this run": it POSTed
`run_started` for a run that does not exist yet, 404'd, failed the handler,
and retried indefinitely. The probe never answered and the cross-deployment
`start()` timed out — which also regressed the pre-existing capability
detection, not just the new key lookup.
Order the health-check member first; it requires `__healthCheck: true`, which
no invoke or step payload carries, so invoke and step payloads still resolve
to their own members.
Also reorder `getPhysicalQueueName` to match health checks before the runId
branch, so under `WORKFLOW_SEQUENTIAL_REPLAYS=1` a probe keeps its per-probe
topic instead of queueing behind the run it is preparing.
karthikscale3
left a comment
karthikscale3
left a comment
There was a problem hiding this comment.
ai review: Approved. The current head correctly mints the final run ID before the cross-deployment probe, derives the public key inside the target deployment, validates the returned key before selecting the write-only sealed path, and preserves the existing symmetric fallback. The runId-bearing health-check union/routing regression is fixed and covered by tests. The remaining webhook E2E timeouts are unrelated: the local case is same-deployment (no probe), and the Vercel run completed while the response-side test wait timed out.
PR 7 of 7. Stacked on #3098; 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 last lookup
After #3096 and #3098, cross-deployment
resumeHook()and forwarded stream writes no longer hit the key API. Cross-deploymentstart()still did — the run doesn't exist yet, so there's no run entity to read a public key from.But
start()already blocks on a capability probe for every cross-deployment call, and the probe responder executes inside the target deployment, where the run's key material is local. So the public key can ride back on a response the caller is already awaiting.Why this beats keeping the request
Ordering change
runIdis now minted before the probe instead of just after.createRunId()reads onlyopts, which is fully resolved at that point, so the move has no other dependency. A test asserts the id sent to the probe is the one actually created, so the ordering can't silently regress.Everything is best-effort
The probe is already failure-tolerant (2s timeout, errors swallowed) and is skipped entirely for same-deployment starts and for worlds without a streams API. When no key comes back — old target, timeout, encryption disabled, or a malformed value —
start()falls back to the existing lookup plus symmetric encryption.Key derivation failures inside the responder are caught and logged so the probe still reports health and capabilities, which callers rely on for reasons unrelated to encryption.
This widens the recipient-substitution surface slightly. The key-lookup API is strongly authenticated and audit-logged; the probe response stream is not. An attacker able to write to that stream — which requires tenant-scoped stream write access — could substitute a public key and read arguments subsequently sealed to it.
That is the same class and roughly the same prerequisite as substituting the key on the run entity (which #3095 already introduces), but it is a second instance of it, and worth weighing explicitly if we later decide public-key attestation is warranted.
Tests
Responder side: key returned when the probe names a run, omitted when it doesn't (with no derivation attempted), omitted when encryption is off, and health still reported when derivation throws.
Caller side: the probe is asked about the run that actually gets created, the arguments are sealed with the probed key and no lookup happens, the sealed arguments actually open with the keys the target re-derives, and both fallbacks (no key, malformed key) work.
Core suite 1610 passing; world 78; workspace typecheck clean; zero new lint diagnostics.