Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…S_SECRET Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ut new options Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…secret Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rcel-platform Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f487791 to
888794b
Compare
chriswdmr
left a comment
chriswdmr
left a comment
There was a problem hiding this comment.
Thanks! Three things to fix before merge:
- SvelteKit without a secret: a failure to load the key must not break the page render
- Add a timeout to the key set fetch
- Next.js middleware: remove the two new Edge Runtime build warnings
The other comments are suggestions and nits.
| }): Handle { | ||
| return async function handle({ event, resolve }) { | ||
| secret ??= await tryGetSecret(secret); | ||
| secret ??= await getSecret(); |
There was a problem hiding this comment.
issue (blocking): A key load failure breaks the page
Without a secret, _encryptFlagValues in transformPageChunk can throw. A 503 or a network error from the key host rejects the handle for each page that uses a flag. The error stays for the 30 s cooldown. A missing project ID has the same result. The values are only for the toolbar. Can we catch the error on this path, leave out the script tag, and log one warning?
This is not a regression: before, createHandle threw without a secret. But it is the weak point of the new mode.
| try { | ||
| // Read the global fetch at each load, because a framework can replace | ||
| // it. no-store, so a framework which caches fetch never keeps old keys. | ||
| const response = await globalThis.fetch(url, { |
There was a problem hiding this comment.
issue (blocking): No timeout on the key set fetch
If the key host does not respond, the render waits for the default of the runtime, which is 300 s in Node.js. Concurrent requests share pending, so they all wait. A key older than 5 minutes is not served during the reload, so warm instances wait too. prepare-flags-definitions uses AbortSignal.timeout(5_000) for its fetch. Can we do the same here?
|
|
||
| function readLinkedProjectId(): string | undefined { | ||
| // getBuiltinModule needs no import, so runtimes other than Node.js work | ||
| const fs = process.getBuiltinModule?.('node:fs'); |
There was a problem hiding this comment.
issue (blocking): New Edge Runtime build warnings
process.getBuiltinModule and process.cwd cause warnings in the Next.js webpack build when middleware.ts imports flags/next. This also occurs with FLAGS_SECRET set. I built tests/next-15: main gives 0 warnings, and this branch gives 2:
A Node.js API is used (process.getBuiltinModule at line: 35) which is not supported in the Edge Runtime.
A Node.js API is used (process.cwd at line: 38) which is not supported in the Edge Runtime.
This change gave 0 warnings in my local build:
const proc = globalThis.process;
const fs = proc?.getBuiltinModule?.('node:fs');
// ...
fs.readFileSync(`${proc.cwd()}/.vercel/project.json`, 'utf8'),| loadedAt = Date.now(); | ||
| error = undefined; | ||
| return value; | ||
| } catch (cause) { |
There was a problem hiding this comment.
issue (non-blocking): The cache hides the control-flow errors of Next.js
In Next.js 15, a static page that calls encryptFlagValues without a secret fails next build. The fetch uses cache: 'no-store', so Next.js throws DynamicServerError to make the page dynamic. This catch wraps the error, the digest is lost, and the build stops. The example in core.mdx is such a page. With a secret, the same page builds as static.
With this line before the wrap, my local build passed and the page became dynamic:
if (cause instanceof Error && 'digest' in cause) throw cause;| */ | ||
| export function findVercelProject(): VercelProject { | ||
| const env = process.env; | ||
| const oidc = readOidcClaims(); |
There was a problem hiding this comment.
nit (non-blocking): Work on each call
readOidcClaims() runs also when VERCEL_PROJECT_ID is set. When the environment has no project ID, .vercel/project.json is read with readFileSync on each call: three calls gave three reads in my test. Could we decode the token only when it is necessary, or memoize the project?
| ) { | ||
| if (!secret) throw new Error('flags: Missing FLAGS_SECRET'); | ||
| return encryptJwe({ v: flagValues, pur: 'values' }, secret, expirationTime); | ||
| if (!secret) return encryptFlagValuesForPlatform(flagValues, expirationTime); |
There was a problem hiding this comment.
suggestion (non-blocking): Say that the fallback is silent
Before, encryptFlagValues(values, process.env.TYPO) or an empty secret threw an error. Now it uses the public key, and decryptFlagValues cannot read that token. Could the changeset and the JSDoc say this?
| }): Handle { | ||
| return async function handle({ event, resolve }) { | ||
| secret ??= await tryGetSecret(secret); | ||
| secret ??= await getSecret(); |
There was a problem hiding this comment.
nit (non-blocking): SvelteKit docs
sveltekit.mdx still shows secret: string as necessary for createHandle. Could we update it in this PR?
| }); | ||
| }); | ||
|
|
||
| it('does not serve the flags discovery endpoint', async () => { |
There was a problem hiding this comment.
suggestion (non-blocking): Two more tests
- The changeset says that the overrides cookie is ignored without a secret. A test for that would be good
- A test for the
.vercel/project.jsonsource (readLinkedProjectId)
Why
encryptFlagValuesthrows withoutFLAGS_SECRET, so a project needs a secret just to keep its flag values confidential.flags-explorer-api-decrypt).standardbranch ([flags] FlagValues: projectIds in the protected header, flags-keys JWKS #575) tomain, with the signature ofencryptFlagValuesunchanged.What
FLAGS_SECRET), nothing changes.encryptFlagValuesencrypts with the public key fromhttps://keys.vercel-flags.com/.well-known/jwks.json(ECDH-ES, A256GCM,projectIdsandkidin the protected header,pur: flag-values,ver: 1,aud: vercel:flag-values). This is the format that@api/flag-valuesdecrypts.VERCEL_PROJECT_ID, theVERCEL_OIDC_TOKENofvercel env pull, or.vercel/project.json.createHandleandencryptFlagValuesno longer require a secret. Without one, thedata-flag-valuesof a page use the public key, the overrides cookie is ignored,/.well-known/vercel/flagsresponds with 401, and precompute still requires a secret.Validation
flag-values-platform.test.tscover the token format, finding the project ID, key caching and rotation, the error cooldown, and that a setFLAGS_SECRETis still used.without-secret.test.tscovers SvelteKit'screateHandleandencryptFlagValueswithout a secret.🤖 Generated with Claude Code