Skip to content

[enh] allow safari-web-extension CORS preflight - refs #49 - #692

Merged
asciimoo merged 3 commits into
asciimoo:masterfrom
ad3lre:safari-extension-cors
Sep 3, 2026
Merged

[enh] allow safari-web-extension CORS preflight - refs #49#692
asciimoo merged 3 commits into
asciimoo:masterfrom
ad3lre:safari-extension-cors

Conversation

@ad3lre

@ad3lre ad3lre commented Aug 29, 2026

Copy link
Copy Markdown

Safari's extension popup still sends a CORS preflight (OPTIONS) for JSON bodies and X-Access-Token. Those requests currently fall through to the SPA handler, so token auth from the unofficial Safari extension fails even though CSRF already allows safari-web-extension:// origins on a subset of paths.

This adds a small middleware that, only for trusted browser-extension origins on the extension API surface:

  • answers OPTIONS with 204
  • reflects that origin in Access-Control-Allow-*
  • skips CSRF on /api/history as well (the content script posts search-result history there; Chrome/Firefox already share this origin check)

Trusted origins are unchanged: moz-extension://, safari-web-extension://, and the packaged Chrome extension id. Other origins are not given CORS headers.

Test plan

  • go test ./server/ -run TestExtension
  • OPTIONS /api/add from a safari-web-extension:// origin returns 204 with Access-Control-Allow-Origin set to that origin
  • the same OPTIONS from https://evil.example does not get those headers
  • POST /api/add and POST /api/history from the Safari origin succeed (201 / 200) with an access token

Assistance: Cursor was used while writing the middleware and tests. The behaviour follows issue #49 and the earlier CORS POC in #46.

Safari still preflights JSON/token requests from the popup. Answer OPTIONS
and skip CSRF on the same extension API surface Chrome and Firefox already
use, including /api/history.
@ad3lre ad3lre mentioned this pull request Aug 29, 2026
@asciimoo

Copy link
Copy Markdown
Owner

Thanks for your contribution.

@nburns could you help verifying the issue/solution?

@ad3lre

ad3lre commented Aug 29, 2026

Copy link
Copy Markdown
Author

I hope to be a long term contributor on here as i have interest in Hister and supporting its compatibility on Safari as an avid Safari user myself

please let me know if anything goes wrong / if I did.

@nburns nburns left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks reasonable to me

Comment thread server/extension.go Outdated
h := w.Header()
h.Set("Access-Control-Allow-Origin", origin)
h.Set("Access-Control-Allow-Methods", "GET, HEAD, POST, OPTIONS")
h.Set("Access-Control-Allow-Headers", "Content-Type, X-Access-Token, X-Hister-Public, X-CSRF-Token, Authorization, Cookie")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ITT Cookie gets removed by the browser/cant be set from JS so we can remove this?

https://developer.mozilla.org/en-US/docs/Glossary/Forbidden_request_header

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yea you're right, I'll get rid of it

Cookie is a forbidden request header in fetch; browsers never send it
via Access-Control-Request-Headers, so listing it in Allow-Headers is
unnecessary.
@asciimoo

asciimoo commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Thanks for both the review and the changes.

Another issue: the fixed header allowlist breaks custom headers. The extension supports arbitrary headers for e.g. reverse proxy authentication, but the middleware permits only a fixed set in Access-Control-Allow-Headers.

Safari lists the actual request headers in Access-Control-Request-Headers;
echoing that list keeps user-configured reverse-proxy headers working instead
of blocking anything outside a fixed allowlist.
@asciimoo
asciimoo merged commit 93dc78f into asciimoo:master Sep 3, 2026
8 checks passed
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.

3 participants