Add AES-256-GCM encryption primitives and HKDF key derivation#956
Add AES-256-GCM encryption primitives and HKDF key derivation#956TooTallNate merged 12 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: c28baee The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 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 Summary
❌ Failed Tests🌍 Community Worlds (45 failed)turso (45 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
|
There was a problem hiding this comment.
Pull request overview
This PR adds AES-256-GCM encryption with HKDF-SHA256 key derivation to the @workflow/world-vercel package. The implementation provides per-run encryption isolation by deriving unique keys from a deployment key, project ID, and run ID. It integrates seamlessly with the async serialization infrastructure added in PR #955 and uses the client-generated run IDs from PR #954.
Changes:
- Implements
createEncryptor()andcreateEncryptorFromEnv()functions with full Encryptor interface support - Adds AES-256-GCM encryption with random nonces and 128-bit authentication tags
- Uses HKDF-SHA256 for per-run key derivation with projectId and runId as context
- Wires encryption into
createVercelWorld()via environment variables (VERCEL_DEPLOYMENT_KEY, VERCEL_PROJECT_ID) - Includes 18 comprehensive tests covering round-trip, format validation, isolation, and tamper detection
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
| packages/world-vercel/src/encryption.ts | Core encryption implementation with key derivation, encrypt/decrypt, and key material access |
| packages/world-vercel/src/encryption.test.ts | Comprehensive test suite with 18 tests covering functionality and security properties |
| packages/world-vercel/src/index.ts | Integration into World creation and public API exports |
Comments suppressed due to low confidence (1)
packages/world-vercel/src/index.ts:12
- The VercelEncryptionConfig interface is exported from encryption.ts but not re-exported from index.ts. Users who want to use createEncryptor() directly would need to import from the internal encryption module, which is not a typical pattern.
Consider adding to index.ts:
export type { VercelEncryptionConfig } from './encryption.js';
This follows the pattern already established with APIConfig and makes the public API more discoverable.
export { createEncryptor, createEncryptorFromEnv } from './encryption.js';
export { createQueue } from './queue.js';
export { createStorage } from './storage.js';
export { createStreamer } from './streamer.js';
export type { APIConfig } from './utils.js';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
d880bc8 to
162847a
Compare
efd031c to
c1795c7
Compare
162847a to
ede75a0
Compare
c1795c7 to
60fac27
Compare
07fb637 to
f32020d
Compare
f86190e to
3201957
Compare
pranaygp
left a comment
There was a problem hiding this comment.
Review: PR #956 - Add Vercel encryption implementation (AES-256-GCM with HKDF)
Summary: Implements AES-256-GCM encryption with HKDF-SHA256 per-run key derivation in @workflow/world-vercel. This is the core crypto PR and the most security-sensitive in the stack.
Crypto Design Review
Algorithm choices are sound:
- AES-256-GCM: authenticated encryption, industry standard, hardware-accelerated
- HKDF-SHA256 for key derivation: correct use of KDF for deriving per-run keys from a deployment key
- 12-byte (96-bit) nonce: standard for GCM
- 128-bit auth tag: standard for GCM
- Random nonce per encryption: correct, avoids nonce reuse
Key derivation design:
HKDF(key=deploymentKey, salt=zeros, info="${projectId}|${runId}")- Zero salt is acceptable here since the deployment key is already random key material (HKDF specification allows this)
- Using
projectId|runIdas info provides good domain separation between runs and projects
Concerns
-
Zero salt vs random salt: While zero salt is acceptable per RFC 5869 when the input key material is already uniformly random, using a random salt stored alongside the ciphertext would provide additional protection against certain attacks (e.g., if the deployment key has less entropy than expected). This is a minor concern given that
VERCEL_DEPLOYMENT_KEYshould be high-entropy, but worth considering for defense in depth. -
Key derivation per encryption call: The
deriveKeyfunction is called on everyencrypt()anddecrypt()call. For workflows with many steps, this means the same HKDF derivation is repeated for each serialization. Consider caching the derived key per(projectId, runId)pair:// Cache derived keys to avoid re-deriving per encrypt/decrypt call const keyCache = new Map<string, CryptoKey>();
The cache key would be
${projectId}|${runId}. This is a performance optimization, not a correctness issue. -
getKeyMaterialreturns raw key bytes: ThegetKeyMaterial()method returns the rawdeploymentKeybytes. This is documented as being for external decryption (o11y tooling), but it means any code with access to the encryptor can extract the master key. Consider:- Adding a warning comment about this being sensitive
- Possibly scoping the returned key material to the derived per-run key rather than the deployment key
-
createEncryptorFromEnvvalidation: WhenVERCEL_DEPLOYMENT_KEYis set but decodes to fewer than 32 bytes,createEncryptorwill throw. However,createEncryptorFromEnvdoesn't catch this, meaning the world creation will fail hard. Consider catching and logging a warning instead, falling back to no encryption, so a misconfigured key doesn't crash the entire application. -
Format prefix
encris 4 bytes of overhead per encrypted payload. This is fine for data payloads but for stream chunks (which may be small), it adds overhead per chunk. Not a major concern but worth noting.
Test Coverage
The 18 tests are comprehensive:
- Round-trip tests (basic, empty, large, all byte values)
- Format validation (prefix, structure, nonce randomness)
- Per-run key isolation
- Tamper detection
- Cross-project/cross-key isolation
- Environment variable handling (missing vars, valid vars)
One test case that could be added: concurrent encryption -- verifying that multiple concurrent encrypt() calls with the same encryptor produce correct results (nonce uniqueness under concurrency).
Minor
- The
Buffer.from(deploymentKeyBase64, 'base64')increateEncryptorFromEnvwill silently ignore invalid base64 characters. Consider validating the decoded length matches expectations, or at minimum letcreateEncryptor's 32-byte check catch it.
Overall, the crypto implementation is solid and follows best practices. The concerns above are mostly optimizations and hardening suggestions rather than security issues.
pranaygp
left a comment
There was a problem hiding this comment.
Overall the crypto design is solid — AES-256-GCM with HKDF-SHA256 per-run key derivation is the right approach, the separation between browser-compatible core encryption and Node.js-specific key management is clean, and the World interface integration is well thought out.
A few concerns below, mostly around performance during replay and a potentially unreachable code path.
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
| if (encryptionKeyOverride) { | ||
| encryptionKey = encryptionKeyOverride; | ||
| } else { | ||
| const rawKey = await world.getEncryptionKeyForRun?.(workflowRun); |
There was a problem hiding this comment.
For cross-deployment hook resumptions / fetching of the encryption key, we should consider enriching the "source" of the request for the key (i.e. resumeHook in this case) for the audit log to include the context.
…llers to import once per run
…oids redundant lookup)
…etEncryptionKeyForRun
…rkflowRun in resume-hook

Summary
Adds the encryption foundation for end-to-end encryption of workflow user data:
@workflow/core: Browser-compatibleencrypt()/decrypt()using Web Crypto API (AES-256-GCM),importKey()for callers to convert raw bytes toCryptoKeyonce per run@workflow/world: OverloadedgetEncryptionKeyForRuninterface — acceptsWorkflowRunentity when available, or(runId, context)forstart()when the run doesn't exist yet@workflow/world-vercel:deriveRunKey()(HKDF-SHA256),fetchRunKey()(API call withprojectId+runId),getEncryptionKeyForRunimplementation readingcontext?.deploymentIdimportKey()after key resolution and passWorkflowRunentity where availablehandleSuspensionrefactored to acceptWorkflowRunparameterresume-hook.tsusesgetHookByTokenWithKeyto fetch fullWorkflowRunalongside hook + key@workflow/core,@workflow/world,@workflow/world-vercel