Skip to content

fix(routing): apply authored headers without overwriting auth cache policy - #669

Open
dvkam wants to merge 5 commits into
voidzero-dev:mainfrom
dvkam:fix/579-static-asset-header-overrides
Open

dvkam wants to merge 5 commits into
voidzero-dev:mainfrom
dvkam:fix/579-static-asset-header-overrides

Conversation

@dvkam

@dvkam dvkam commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Fixes #579

Apply routing header rules through Hono's header API so configured values replace existing response headers. Keep authentication in control of private and no-store caching, including sign-in responses reached through rewrites or fallbacks.

Preserve last-match precedence and document that header rules also apply to matching error responses.

Validation:

  • Reproduced the CSS and JavaScript failure in build and preview, then verified the fix.
  • 188 focused routing, auth, compiler and real asset-binding tests pass.
  • Workspace type checks pass, 33 tasks. Scoped formatting and lint pass.
  • Full suite: 15,372 passed and 20 failed. Eighteen failures also reproduce on the base revision. The other two pass in isolation.

@dvkam
dvkam marked this pull request as ready for review November 2, 2026 10:55
@Brooooooklyn
Brooooooklyn self-requested a review November 3, 2026 06:34
Brooooooklyn and others added 2 commits November 3, 2026 14:59
… headers

- Refresh the static-asset mark after a rule sets Cache-Control, so auth
  makes the authored file policy private for credentialed requests
  instead of replacing it with no-store.
- Append Set-Cookie rules so the response's own cookies, including the
  Better Auth session, are kept.
- Only count Better Auth cookies when deciding that a response needs the
  auth cache policy; app cookies keep the route's own policy.
- Skip the outer header pass for fallback answers, so only the
  destination's rules apply, as in @void/edge and platform dispatch.
- Docs: say where auth keeps private caching and what a platform does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Brooooooklyn

Copy link
Copy Markdown
Member

Thanks for the fix and the clear write-up. I reviewed it in depth and pushed a follow-up commit, and merged main.

The core change (rules applied through c.header, header middleware inside auth) stays. The commit fixes five cases where the new order or the new "replace" behaviour went wrong:

case                                                      before this commit          now
────────────────────────────────────────────────────────  ──────────────────────────  ─────────────────────────────────────
auth app + any cookie + '/assets/*' immutable rule        no-store (every reload)     private, max-age=31536000, immutable
'/*': Set-Cookie rule on sign-in                          session cookie dropped      both cookies kept ("as written")
auth app, no rules, route sets its own cookie             route policy → no-store     route policy kept
'/*' -> '/index.html' fallback at /assets/old-chunk.js    HTML gets immutable         HTML keeps its own policy
docs: auth keeps private/no-store                         stated for every target     native/local/other runtimes; platform note

How:

  • headerMiddleware refreshes the static-asset mark after a rule changes Cache-Control, so auth still sees a static file and only makes it private for credentialed requests.
  • Set-Cookie rules append instead of replace.
  • The response-cookie trigger in auth now only counts Better Auth cookies (prefix, __Secure-/__Host-, custom cookie names).
  • A fallback answer skips the outer pass: only the destination's rules apply, as in @void/edge and platform dispatch. The recursive-fallback test is back to 'one'.
  • Each case has a test that fails on the previous commit.

Left as is (noted for follow-ups, not part of #579):

  • A 500 raised inside the auth middleware itself (for example a missing secret or database) no longer gets header rules, because the header middleware now sits inside auth.
  • Node/Bun/Deno serve /assets/* before the Hono app, so header rules do not reach those files. This is older behaviour.
  • On a Void platform, dispatch applies header rules after the Worker, so a public Cache-Control rule can replace auth's policy. The docs now say so.

@Brooooooklyn

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T11:48:10.858711Z 933062d Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 933062d109

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +665 to +667
setsAuthCookie(c.res.headers, auth.options.advanced) ||
c.req.path === '/api/auth' ||
c.req.path.startsWith('/api/auth/'),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve auth cache protection through cookie-free rewrites

In a native/local app with a public cache rule, an anonymous GET /session implemented as c.rewrite('/api/auth/get-session') loses its auth cache protection. The destination correctly emits no-store, but the source request's headerMiddleware overwrites it. These checks then see neither an auth source path nor a response cookie, so the outer auth middleware skips restoring the policy. The final auth response advertises the configured public TTL, allowing downstream caches to retain the anonymous session result. Propagate auth-response identity through recursive rewrites rather than inferring it only from the source path or Set-Cookie.

Useful? React with 👍 / 👎.

Comment on lines +314 to +315
if (emitEdgeRules && options.headerRules?.length) {
compileHeaderMiddleware(lines, options.headerRules);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Apply authored headers when auth fails before next

If auth initialization or auth.api.getSession() throws before next()—for example, on a transient database failure—registering the header middleware after auth means it is never entered. createErrorRenderer.onError returns the error response without applying header rules, so a configured Access-Control-Allow-Origin is absent and cross-origin clients cannot read the error response; previously the outer header middleware still decorated it. Keep non-cache header finalization outside this failure boundary, or explicitly apply it to auth error responses, while retaining auth's control over the final cache policy.

Useful? React with 👍 / 👎.

This branch has not been deployed

No deployments
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.

routing.headers cannot set Cache-Control on static assets

2 participants