Skip to content

feat(middleware): don't cache responses with a Set-Cookie header - #806

Open
u5surf wants to merge 2 commits into
darkweak:masterfrom
u5surf:feat/skip-store-set-cookie
Open

u5surf wants to merge 2 commits into
darkweak:masterfrom
u5surf:feat/skip-store-set-cookie

Conversation

@u5surf

@u5surf u5surf commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

A Set-Cookie response is typically tied to a single user; storing it in a shared cache replays one user's cookie to every subsequent client. Match the default of nginx/Varnish/Fastly/Cloudflare and skip storing such responses (detail=UNCACHEABLE-SET-COOKIE).

Closes #805

@netlify

netlify Bot commented Jun 12, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for teal-sprinkles-4c7f14 canceled.

Name Link
🔨 Latest commit 15bae64
🔍 Latest deploy log https://app.netlify.com/projects/teal-sprinkles-4c7f14/deploys/6a3d0cd836cf7700083ca3ec

@darkweak

Copy link
Copy Markdown
Owner

Can you rebase @u5surf?

@u5surf
u5surf force-pushed the feat/skip-store-set-cookie branch 2 times, most recently from 03fc2ae to 15bae64 Compare June 25, 2026 11:11
@darkweak

Copy link
Copy Markdown
Owner

The opt-in to keep the current behavior is missing.
Thank you for your contributions!

A Set-Cookie response is typically tied to a single user; storing it in
a shared cache replays one user's cookie to every subsequent client.
Match the default of nginx/Varnish/Fastly/Cloudflare and skip storing
such responses (detail=UNCACHEABLE-SET-COOKIE).

The exception is an explicit Cache-Control: no-cache="Set-Cookie"
directive, which strips the field from the stored response: there is
nothing to leak, so the rest of the response is still cached (preserving
the existing TestSetCookieNotStored behavior).

Closes darkweak#805

Signed-off-by: Y.Horie <u5.horie@gmail.com>
@u5surf
u5surf force-pushed the feat/skip-store-set-cookie branch from 15bae64 to 4a28b8d Compare October 2, 2026 14:01
@netlify

netlify Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for teal-sprinkles-4c7f14 canceled.

Name Link
🔨 Latest commit fcd2a01
🔍 Latest deploy log https://app.netlify.com/projects/teal-sprinkles-4c7f14/deploys/6abfc5a31e41fe0008b863c0

@u5surf

u5surf commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

@darkweak Rebased on master and added the opt-in.

Setting default_cache.cache_set_cookie: true keeps the current behavior: responses with a Set-Cookie header are stored as before. It defaults to false, so these responses aren't stored unless you opt in.

  • Caddyfile: cache { cache_set_cookie }. It works in both the global and handler config, and a handler inherits the global value.
  • Also wired into the agnostic parser and the Kratos plugin, and documented in the configuration reference.
  • Tests: a middleware unit test, plus a Caddy integration test that checks both the default and the opt-in.

I left the Traefik override/vendor copies alone, like the original commit. Let me know if you'd like a different option name or want it there too.

…ponses

Responses carrying a Set-Cookie header are no longer stored by default.
Add a default_cache.cache_set_cookie option to keep the previous
behavior for deployments that rely on it.

The option is wired into the Caddy plugin (global and handler level,
with the handler inheriting the global value), the agnostic parser and
the Kratos plugin, and documented in the configuration reference.

Signed-off-by: Y.Horie <u5.horie@gmail.com>
@u5surf
u5surf force-pushed the feat/skip-store-set-cookie branch from 4a28b8d to fcd2a01 Compare October 2, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't store Set-Cookie responses by default (cookie leak risk)

2 participants