test: enable encryption on nextjs-turbopack workbench#1262
Open
TooTallNate wants to merge 7 commits intomainfrom
Open
test: enable encryption on nextjs-turbopack workbench#1262TooTallNate wants to merge 7 commits intomainfrom
TooTallNate wants to merge 7 commits intomainfrom
Conversation
Set VERCEL_DEPLOYMENT_KEY to enable e2e encryption for the nextjs-turbopack workbench project. This allows the e2e test suite to exercise encrypted workflow runs alongside existing unencrypted runs for backwards compatibility testing.
Contributor
🦋 Changeset detectedLatest commit: 2353fbc The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 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
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (49 failed)mongodb (1 failed):
turso (48 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
|
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Enables e2e encryption for the nextjs-turbopack workbench deployment by configuring VERCEL_DEPLOYMENT_KEY in the Vercel project config so the test suite exercises encrypted payloads and decryption paths.
Changes:
- Adds
VERCEL_DEPLOYMENT_KEYto the workbench’svercel.jsonenvironment configuration.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| "env": { | ||
| "WORKFLOW_PUBLIC_MANIFEST": "1" | ||
| "WORKFLOW_PUBLIC_MANIFEST": "1", | ||
| "VERCEL_DEPLOYMENT_KEY": "6r4/aqeYUXPsPyLozM4OjSiS7N88aeuWYLFMAKiDE94=" |
There was a problem hiding this comment.
VERCEL_DEPLOYMENT_KEY looks like a sensitive deployment secret (used to derive encryption keys) but is being committed as a literal value in vercel.json. This exposes the key in git history and to anyone with repo access, and it will apply to every future deployment of this workbench. Please remove the hard-coded value and source it from Vercel-managed environment variables/secrets instead (e.g., configure it in the Vercel project/preview env, or reference a Vercel secret rather than inlining). Also rotate the key since it’s now been published in the PR diff.
Suggested change
| "VERCEL_DEPLOYMENT_KEY": "6r4/aqeYUXPsPyLozM4OjSiS7N88aeuWYLFMAKiDE94=" | |
| "VERCEL_DEPLOYMENT_KEY": "@vercel-deployment-key" |
Contributor
There was a problem hiding this comment.
This is a false positive. The workbench/nextjs-turbopack project is a private test workbench used for e2e testing — it's not a production application. The key is intentionally committed as a test fixture so that CI deployments of this workbench exercise the encryption code paths. There's no security risk here since this key only protects test data in a test environment.
Member
Author
There was a problem hiding this comment.
It's temporary dude. Read the PR description.
The e2e runner calls fetchRunKey for every run, which can exceed the API rate limit (30/min per team) when many tests run in parallel. Add retry logic with exponential backoff + jitter (up to 5 retries) and respect the Retry-After header when present.
…eout(r, NaN)` to resolve immediately (0ms), triggering rapid-fire retries against a rate-limited API endpoint.
This commit fixes the issue reported at packages/world-vercel/src/encryption.ts:138
**Bug Analysis:**
In `packages/world-vercel/src/encryption.ts`, the `fetchRunKey` function handles HTTP 429 (rate limited) responses by reading the `retry-after` header and computing a delay. The original code was:
```js
const retryAfter = response.headers.get('retry-after');
const delay = retryAfter
? Number.parseInt(retryAfter, 10) * 1000
: BASE_DELAY_MS * 2 ** attempt + Math.random() * 500;
```
Per RFC 9110, the `Retry-After` header can contain either a number of seconds OR an HTTP date string (e.g., `"Wed, 21 Oct 2015 07:28:00 GMT"`). When the header contains a date string or any non-numeric value, `Number.parseInt(retryAfter, 10)` returns `NaN`. Since the header string is truthy (non-empty), the ternary takes the parseInt path rather than the exponential backoff fallback. `NaN * 1000` evaluates to `NaN`, and `setTimeout(r, NaN)` resolves immediately (~0ms, confirmed by testing). This causes rapid-fire retries (up to 5 iterations with zero delay) against a rate-limited API endpoint, wasting resources and potentially worsening the rate-limiting situation.
**Fix:**
Parse the integer first, then check if the result is NaN before using it. If NaN, fall back to exponential backoff with jitter. This matches the established pattern already used in `packages/world-vercel/src/utils.ts` (lines 309-315) where the same header is properly guarded with `!Number.isNaN(parsed)`.
```js
const retryAfterHeader = response.headers.get('retry-after');
const parsedRetryAfter = retryAfterHeader
? Number.parseInt(retryAfterHeader, 10)
: NaN;
const delay = !Number.isNaN(parsedRetryAfter)
? parsedRetryAfter * 1000
: BASE_DELAY_MS * 2 ** attempt + Math.random() * 500;
```
Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: TooTallNate <n@n8.io>
With encryption enabled on the nextjs-turbopack workbench, CLI inspect output shows encrypted data as placeholders. The e2e tests check for actual output values, so they need --decrypt to see the real data.
The shared RetryAgent from getDispatcher() already handles 429/5xx retries with exponential backoff and Retry-After header support. The manual retry loop in fetchRunKey was redundant. Also add changeset for the VERCEL=1 external context fix.
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Enables e2e encryption on the
nextjs-turbopackworkbench project by settingVERCEL_DEPLOYMENT_KEYinvercel.json.Purpose
This triggers the e2e test suite to run with encryption enabled, which validates:
--decryptflag works against real encrypted dataWhat happens
Once this PR's preview deployment is live:
Cleanup
This key should be removed after the encryption bugbash is complete.