Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
luismeyer
changed the title
104bfaf to
ed43299
Compare
On Vercel, or locally after `vercel env pull`, verify override tokens which the Vercel Toolbar signs with Vercel KMS, alongside FLAGS_SECRET tokens. Share the host project lookup and the JWKS cache with the platform encryption of the flag values. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ds group Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lues Rename vercel-overrides to flag-overrides-platform, and use the same shape as flag-values-platform: an exported issuer, JWKS URL and frozen policy, a publicKeys cache, and the FromPlatform/ForPlatform names. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Apps which read the vercel-flag-overrides cookie themselves call decryptOverrides, so it verifies overrides signed by Vercel KMS too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
chriswdmr
left a comment
chriswdmr
left a comment
There was a problem hiding this comment.
Thanks! Two things to fix before approval:
- SvelteKit: signed overrides do not work in local development, because
vite devdoes not put.env.localvalues inprocess.env - Replace the nested ternary in SvelteKit
flag()and add braces to theifinisInScope(AGENTS.md)
The other comments are non-blocking. The "Shared with #586" part and the test count in the description are out of date.
| */ | ||
| export function isVercelSignedToken(token: string): boolean { | ||
| return ( | ||
| Boolean(process.env.VERCEL || process.env.VERCEL_OIDC_TOKEN) && |
There was a problem hiding this comment.
issue: SvelteKit local development never accepts a signed token
In vite dev, values from .env.local are in $env/* but not in process.env. That is why sveltekit/env.ts imports $env/static/private for the secret. This check and findVercelProject() read process.env only. So after vercel env pull, a SvelteKit app still ignores each signed token locally, with no log.
I checked this in examples/sveltekit-example with a temporary env file and route: process.env.VERCEL and process.env.VERCEL_OIDC_TOKEN were not set, and $env/dynamic/private had both.
Could the SvelteKit entry supply these variables, as it does for the secret? If not, the changeset must say that the local case is for Next.js only. findVercelProject() comes from #586, so the fix can go there.
| const overrides = !overridesCookie | ||
| ? undefined | ||
| : isFlagOverridesFromPlatform(overridesCookie) | ||
| ? await verifyFlagOverridesFromPlatform(overridesCookie) | ||
| : store.secret | ||
| ? await _decryptOverrides(overridesCookie, store.secret) | ||
| : undefined; |
There was a problem hiding this comment.
issue: Nested ternary
AGENTS.md says to avoid nested ternaries. _decryptOverrides already sends a signed token to the verifier, so one guard is sufficient:
let overrides: FlagOverridesType | undefined;
if (
overridesCookie &&
(store.secret || isFlagOverridesFromPlatform(overridesCookie))
) {
overrides = await _decryptOverrides(overridesCookie, store.secret);
}I tried this locally: the type check and the SvelteKit tests pass.
| scope: unknown, | ||
| host: { projectId: string; environment: string; teamId?: string }, | ||
| ): boolean { | ||
| if (!isRecord(scope)) return false; |
There was a problem hiding this comment.
issue: Missing braces
AGENTS.md asks for curly braces on each control-flow body in new code:
if (!isRecord(scope)) {
return false;
}| const cookieOverrides = isFlagOverridesFromPlatform(cookie) | ||
| ? await verifyFlagOverridesFromPlatform(cookie) | ||
| : await memoizedDecrypt(cookie); |
There was a problem hiding this comment.
suggestion (non-blocking): Ignore a cookie that the host can not read when there is no secret
This is not new. On main, each non-empty cookie makes flag() throw flags: Missing FLAGS_SECRET when there is no secret. But this PR makes an app with no secret a supported setup, and there the throw still occurs in two cases:
- A signed cookie when the host can not verify it, e.g. locally before
vercel env pull - An older encrypted cookie that is still in the browser after the project removes
FLAGS_SECRET
The message then points to the wrong fix. SvelteKit flag() ignores the cookie in the same cases. Could Next do the same?
| } catch { | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
suggestion (non-blocking): Log why a signed token is ignored
A signed cookie that the host can not use is ignored with no log. The user then sees no override and no reason. Cases that I checked:
- A local Next app with
FLAGS_SECRETin its env file but novercel env pull(the token goes to the decrypt path and fails there) - The key set does not load
- The token is for a different project or environment
A console.warn (not more than one per cooldown) that names the cause would help. For the first case, it can name vercel env pull as the fix.
| publicKeys.set(issuer, keys); | ||
| return async (header, token) => { | ||
| try { | ||
| return await (await keys.get())(header, token); |
There was a problem hiding this comment.
suggestion (non-blocking): The key fetch has no timeout
createKeySetCache (from #586) calls fetch with no signal. With this PR, a flag call for a request with a signed cookie waits on that fetch. If the key set endpoint does not answer, the flag call does not settle. This also occurs when keys are in memory but older than 5 minutes, because get() waits for the reload.
An AbortSignal.timeout(...) on the fetch, and the keys in memory as the result during a reload, would fix it. This can go in #586.
| }, | ||
| }), | ||
| ], | ||
| ])('rejects %s', async (_, token) => { |
There was a problem hiding this comment.
suggestion (non-blocking): Some more rejection cases
The verifier rejects these correctly (I checked locally), and the policy depends on them, but they have no test:
- A future
iatand a futurenbf - A missing
iatand a missingexp - A token longer than
maxTokenLength x5uandcritin the header
| const keys = | ||
| publicKeys.get(issuer) ?? | ||
| createKeySetCache( | ||
| `${issuer}/jwks.json`, | ||
| 'the public keys of Vercel KMS', | ||
| (keySet) => createLocalJWKSet(keySet as unknown as JSONWebKeySet), | ||
| ); | ||
| publicKeys.set(issuer, keys); |
There was a problem hiding this comment.
nitpick (non-blocking): Set the cache entry only when it is missing
publicKeys.set runs on each call, also when the entry exists:
let keys = publicKeys.get(issuer);
if (!keys) {
keys = createKeySetCache(/* ... */);
publicKeys.set(issuer, keys);
}|
|
||
| /** | ||
| * Whether the token is signed by Vercel (a JWS, which has three parts) and the | ||
| * host can verify it: on Vercel, or locally after `vercel env pull`. |
There was a problem hiding this comment.
question (non-blocking): What occurs when system environment variables are not exposed?
The docs describe VERCEL as "An indicator to show that system environment variables have been exposed to your project's Deployments". If a project has that setting off, I think this check is false on Vercel, and VERCEL_PROJECT_ID is also missing. Is that correct? If yes, a note in the docs or the changeset would help.
Summary
Lets v4 apps accept flag overrides that the Vercel Toolbar signs with Vercel KMS, so overrides work on Vercel without a
FLAGS_SECRET. This is the SDK half of vercel/next-live-mode#3898.vercel-flag-overrides, and no exports change.VERCEL), or locally aftervercel env pull(VERCEL_OIDC_TOKEN), a 3-part JWS is verified as a Vercel-signed token. Everything else goes to the existingdecryptOverrides, unchanged. Signed andFLAGS_SECRETtokens are dual-read, so the Toolbar can roll signing out to projects that still have aFLAGS_SECRET.src/lib/flag-overrides-platform.ts, shaped likeflag-values-platform.ts: exported issuer, JWKS URL and frozenFLAG_OVERRIDES_POLICY;isFlagOverridesFromPlatform/verifyFlagOverridesFromPlatform):https://kms.vercel.com/64af96a2-ba60-42ef-91af-459fcef6f864; audvercel:flag-overrides; purflag-overrides; ver 1; requirediat/exp; at most 7-day lifetime.kidand nojku/jwk/x5u/crit. The token is limited to 16 KB.scope.projectIds(all projects of a microfrontends group share the cookie), its environment inenvironments, and the team must match when known. Unknown scope keys are rejected.projectIdsdiffers fromscope.projectIdin the v5 standard, which needs the same change.src/lib/vercel-platform.ts):findVercelProject(): the host project fromVERCEL_PROJECT_ID/VERCEL_TARGET_ENV, theVERCEL_OIDC_TOKENclaims, or.vercel/project.json.createKeySetCache(): a JWKS fetched withno-storeand kept in memory for 5 minutes. Concurrent loads share one request, and it reloads at most once per 30s for an unknownkidor after an error. It keeps the size, key-count and private-key checks.flag-values-platform.tsfrom [flags] Encrypt FlagValues with the public key of Vercel without FLAGS_SECRET #586 now uses both. Its behavior is unchanged, except that a failed reload keeps serving the last good key instead of throwing.Notes for reconciling with
standard(v5)standardrejects signed tokens whenFLAGS_SECRETis set. This PR intentionally accepts them, because the Toolbar signs for all projects behind its rollout flag. v5 needs the same rule before it ships.standardreadsflag-overridesfirst, thenvercel-flag-overrides, so v5 already works with the Toolbar writingvercel-flag-overrides.standard'svercel.ts. This module is a reduced copy: no options object, and ES256 only.Test plan
pnpm type-check,pnpm check,pnpm buildandpnpm attwpasspnpm test: 197/197 pass, including [flags] Encrypt FlagValues with the public key of Vercel without FLAGS_SECRET #586's tests unchanged and a secret-less SvelteKit signed-override test🤖 Generated with Claude Code