Skip to content

fix: stop 404s/redirects from being publicly CDN-cached #2526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
agalin920 wants to merge 1 commit into
base: stage
Choose a base branch
from
Open

fix: stop 404s/redirects from being publicly CDN-cached #2526

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions src/pages/[[...zesty]].js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,11 @@ export async function getServerSideProps({ req, res, resolvedUrl }) {
// does not display with npm run dev

res.setHeader('set-cookie', `PRODUCTION=${process.env.PRODUCTION}`);
isProd &&
res.setHeader(
'Cache-Control',
'public, max-age=3600, stale-while-revalidate=7200 ',
);
isProd && res.setHeader('Surrogate-Control', 'max-age=3600');

res.setHeader(
'Surrogate-Key',
`${process.env.zesty.instance_zuid}, zesty.io`,
);
// Default to non-cacheable; cache headers are set only on the 200 path below
// so 404s/redirects can't be pinned at the CDN edge and served to everyone.
res.setHeader('Cache-Control', 'private, no-store');

@github-actions github-actions Bot Jul 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small behavior change worth calling out explicitly: previously Surrogate-Key was set unconditionally (outside the isProd && guards), so dev/stage responses also carried it. With this refactor, Surrogate-Key is only emitted on the successful isProd 200 path. That's very likely intentional (surrogate keys are only meaningful behind Fastly), but if any stage-facing tooling or purge script currently relies on the header being present in stage, it will silently stop working. Worth a quick confirm.

Also, per your own verification table, Next.js overrides the header on the notFound path to no-store, must-revalidate — so the private, no-store default here is really only observed on the redirect path and any thrown-500 path. Still correct, just noting the default set here isn't what actually goes out for 404s.

@agalin920 agalin920 Jul 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct and intentional. As I mentiond in the PR desc: Surrogate-Control/Surrogate-Key are the operative headers here. On Fastly, Surrogate-Control controls the shared edge cache and overrides Cache-Control. Next already forced Cache-Control: no-store on the 404, but it doesn't touch the surrogate headers, so Fastly obeyed Surrogate-Control and edge-cached the 404 anyway. The fix's key effect is dropping the surrogate headers on non-200 paths, not the Cache-Control default.

On stage it's inert: isProd already gated the public Cache-Control, so stage responses were never shared-cacheable regardless of the key


// Fetch the page data using the cache function
let data = await fetchPage(resolvedUrl);
// attempt to get page data relative to zesty
Expand Down Expand Up @@ -160,6 +154,19 @@ export async function getServerSideProps({ req, res, resolvedUrl }) {
};
}

// Real 200 page: safe to cache and tag with the instance Surrogate-Key.
if (isProd) {

@github-actions github-actions Bot Jul 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice side effect worth noting: the two auth-driven redirects above (/ → /dashboard/ for logged-in users, and /login/ → / for logged-in users) had the same bug as the 404 case described in the PR — under the old code those 307s went out public, max-age=3600 and tagged with the instance Surrogate-Key. In principle an anonymous user hitting / right after an authenticated user could have gotten the cached 307 to /dashboard/, or vice versa. Since they now sit above this isProd block, they inherit the private, no-store default and are non-cacheable. Might be worth a one-liner in the PR description so reviewers/on-call know the fix is broader than the /instances/ 404 symptom.

@agalin920 agalin920 Jul 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, same bug class as the 404. these 307s carried Surrogate-Control + the instance Surrogate-Key, so Fastly could edge-cache an auth-dependent redirect and serve it to the wrong user.

res.setHeader(
'Cache-Control',
'public, max-age=3600, stale-while-revalidate=7200',
);
res.setHeader('Surrogate-Control', 'max-age=3600');
res.setHeader(
'Surrogate-Key',
`${process.env.zesty.instance_zuid}, zesty.io`,
);
}

// Pass data to the page via props
return { props: { ...data } };
}
Loading