fix(cookies): prevent mutation outside action phase#91069
fix(cookies): prevent mutation outside action phase#91069Neiland85 wants to merge 1 commit intovercel:canaryfrom
Conversation
|
Allow CI Workflow Run
Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer |
There was a problem hiding this comment.
Pull request overview
Adjusts cookie mutation behavior in the server request cookie adapters to ensure cookies cannot be mutated outside the Server Action (“action”) phase.
Changes:
- Adds phase checks to
MutableRequestCookiesAdapter.wrap()to blockset/deletewhen cookies are no longer mutable. - Refactors/moves
appendMutableCookieswithin the module (no intended functional change).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const requestStore = workAsyncStorage.getStore() as | ||
| | RequestStore | ||
| | undefined | ||
|
|
||
| if (requestStore) { | ||
| ensureCookiesAreStillMutable(requestStore, 'cookies().delete') | ||
| } |
There was a problem hiding this comment.
workAsyncStorage.getStore() is typed to WorkStore (app-render work store), not RequestStore. Casting it to RequestStore and passing it into ensureCookiesAreStillMutable will treat the work store as if it has a phase field, causing cookies().set/delete to throw unexpectedly whenever a WorkStore is present (because phase will be undefined). This should read from workUnitAsyncStorage.getStore() (and then narrow to type === 'request') or otherwise obtain the actual RequestStore that owns the phase transitions.
| case 'set': | ||
| return function (...args: SetCookieArgs) { | ||
| const requestStore = workAsyncStorage.getStore() as | ||
| | RequestStore | ||
| | undefined | ||
|
|
||
| if (requestStore) { | ||
| ensureCookiesAreStillMutable(requestStore, 'cookies().set') | ||
| } |
There was a problem hiding this comment.
This change introduces phase-dependent mutation checks inside MutableRequestCookiesAdapter.wrap, but there are no unit tests covering the new behavior (e.g., that mutations are allowed in phase === 'action' and rejected after transitioning to render/after). Consider adding tests similar to the existing wrapWithMutableAccessCheck suite to prevent regressions once the correct async storage is used.
Summary
Adds a runtime guard preventing cookie mutations outside the
actionphase.Context
cookies().set()andcookies().delete()currently rely on callers respectingthe mutation phase. This change ensures mutations are validated against the
RequestStore.phaseat runtime before executing.Behavior
If a mutation is attempted outside the allowed phase (
action), the callthrows
ReadonlyRequestCookiesError.Impact