Skip to content

Conversation

@mkhanas
Copy link
Contributor

@mkhanas mkhanas commented Nov 2, 2025

As agreed in #1803 :

  • Updated Fetch Metadata positioning

  • Extended core guidance to mention Fetch Metadata request headers (Sec-Fetch-*) as an alternative to CSRF tokens for state-changing requests.

  • Added clarification that developers can use CSRF tokens or Fetch Metadata depending on project needs and client compatibility.

  • In case of a new Cheat Sheet, you have used the Cheat Sheet template.

  • All the markdown files do not raise any validation policy violation, see the policy.

  • All the markdown files follow these format rules.

  • All your assets are stored in the assets folder.

  • All the images used are in the PNG format.

  • Any references to websites have been formatted as [TEXT](URL)

  • You verified/tested the effectiveness of your contribution (e.g., the defensive code proposed is really an effective remediation? Please verify it works!).

  • The CI build of your PR pass, see the build status here.

AI Tool Usage Disclosure (required for all PRs)

Please select one of the following options:

  • I have NOT used any AI tool to generate the contents of this PR.
  • I have used AI tools to generate the contents of this PR. I have verified
    the contents and I affirm the results. The LLM used is [llm name and version]
    and the prompt used is [your prompt here]. [Feel free to add more details if needed]

This PR fixes issue #1803.

Extended core guidance to mention Fetch Metadata request headers (Sec-Fetch-*) as an alternative to CSRF tokens for state-changing requests.

Added clarification that developers can use CSRF tokens or Fetch Metadata depending on project needs and client compatibility.

Updated Fetch Metadata positionaing
@nickchomey
Copy link

nickchomey commented Nov 2, 2025

Thanks for taking the lead on this! You'll definitely want a review by someone more knowledgeable (eg. @FiloSottile), but here's some feedback.

I believe it was agreed in #1803 that "Fetch metadata is a complete and robust fix for CSRF, not just defense in depth." However, this PR reads unnecessarily precautionary.

  • First, i think it would be easier for people to review if you didn't move the Fetch Metadata headers section. The whole thing is a diff, rather than just specific wording. If it should be moved, that could be done before merging the PR, after approval of the language.
  • Various places call it a "primary" protection, which still suggests it is defense in depth. Such language is not used with respect to the CSRF tokens sections
  • The concerns about browser compatibility are vastly overstated. It has 97.6% coverage, which is practically 100%. Only some ancient Chrome, IE, and 2+ year old Safari (which surely will be updated soon enough) are unsupported. The real security risk is anyone using these browsers - you'd be doing them a favour by returning a message saying "update your browser if you want to use this site". Moreover, Origin headers - which are used by the Go implementation - have effectively 100% coverage
  • non-browser (eg curl) request support is a non-issue
  • It is referred to as "relatively new", "simple" etc - this undermines its robustness.
  • Using Origin headers as the first fallback should be strongly/mandatorily recommended. In fact, it could link to the Go CrossOriginProtection source code as a model implementation.
  • The limitations and gotchas section was not modified at all. Again, browser compatibility is not an issue. There's no substantiation of the concerns about blocking legitimate CORS or 3rd party flows - either these concerns should be listed, along with mitigations, or not mentioned at all. I dont see how it is a limitation that it needs to work with https - that's simply good practice. In fact, HSTS should be encouraged.

@mkhanas
Copy link
Contributor Author

mkhanas commented Nov 4, 2025

Hi, @nickchomey! Thanks a lot for the thorough review and for the pointers, here some thoughts

this PR reads unnecessarily precautionary

You’re right — that’s on me. The phrasing ended up sounding more precautionary than intended; it reflects my personal stance on the topic

First, i think it would be easier for people to review if you didn't move the Fetch Metadata headers section. The whole thing is a diff, rather than just specific wording. If it should be moved, that could be done before merging the PR, after approval of the language.

I moved the Fetch Metadata section because we’re proposing it be treated as one of the primary mitigations. If we leave it buried, readers may miss that change.

Various places call it a "primary" protection, which still suggests it is defense in depth. Such language is not used with respect to the CSRF tokens sections

I understand your point — my intent was to reflect the current state of adoption and confidence rather than to imply it’s inherently secondary.

As I mentioned in the issue discussion, Fetch Metadata is still relatively new compared to traditional CSRF mitigations. While it’s an elegant and highly effective mechanism for modern browsers, it hasn’t yet gone through the same long cycle of real-world testing, widespread deployment, and maturity that CSRF tokens have. In security, mechanisms often take time to earn broad trust — a good parallel might be the SameSite cookie attribute, which was proposed years before browsers began enforcing it by default and before most frameworks integrated it as a standard protection.

That said, I’m open to rephrasing. Which parts stand out to you most?

The concerns about browser compatibility are vastly overstated.

My point wasn’t to question the coverage but rather to acknowledge that, as a community, we can’t dictate every project’s goals or constraints.

If the software targets modern browsers, then Fetch Metadata is clearly the way to go. However, as we discussed, for environments where these headers aren’t yet supported, developers will still need to rely on CSRF tokens — even if that feels less practical.

non-browser (eg curl) request support is a non-issue

Agree

The limitations and gotchas section was not modified at all.

Regarding the note on blocking legitimate CORS or third-party flows — I intentionally kept it, just to highlight that these areas need extra care, since enabling Fetch Metadata protections without considering such flows could break them. The relevant mitigations are already covered in section 3.2 (“How to treat Fetch Metadata headers on the server-side”) section.

In fact, HSTS should be encouraged.

Good catch! I hadn’t considered mentioning it explicitly, but that’s a great recommendation and would indeed address the “potentially trustworthy” issue as well. I’ll incorporate that.

@nickchomey
Copy link

nickchomey commented Nov 6, 2025

I can't help but think that your response was written almost completely by AI... It makes me wary of collaborating any further on this as its not clear to me that a thoughtful human is actually on the other side...

Perhaps incorporate my suggestions into a new commit and we can see where we're at then

@mkhanas
Copy link
Contributor Author

mkhanas commented Nov 6, 2025

I don’t like where this is headed, especially with things starting to feel a bit personal.

It feels like the focus is mainly on getting Fetch Metadata headers recognized as the primary CSRF protection and moving away from CSRF tokens entirely, which doesn’t sit well with me. That’s just my personal view, but I worry it could lead to CSRF tokens falling out of use altogether, since most developers would naturally choose the simpler Fetch Metadata approach.

I get your point though, and as I mentioned in my previous comment, this section reflects my own perspective. It’s admittedly an awkward position — I don’t have solid evidence that Fetch Metadata isn’t as robust as you’d like to present it, but at the same time, “lack of evidence doesn’t mean lack of existence” isn’t a strong argument either. I’m just not sure how to balance both sides — treating Fetch Metadata as “the best” while still keeping CSRF tokens relevant.

@mkhanas
Copy link
Contributor Author

mkhanas commented Nov 6, 2025

Feel free to open your own PR — I’m not comfortable positioning Fetch Metadata as THE MAIN CSRF defense.

@nickchomey
Copy link

nickchomey commented Nov 6, 2025

These last 2 comments, and those from the previous issue, have no hint of AI.

I don't think anyone was ever advocating for fetch metadata being THE primary csrf protection. Just that it is suitable as A Primary/standalone protection, if a few niche caveats and associated mitigation are presented.

 I worry it could lead to CSRF tokens falling out of use altogether, since most developers would naturally choose the simpler Fetch Metadata approach.

Though, this is precisely what people are ultimately advocating for - CSRF tokens are a headache (and therefore much more likely to implemented poorly due to human error). They also make caching extremely difficult.

I see no conflict or even issue with effectively saying "these are two worthy options. Take your pick". Most would, indeed, eventually pick fetch metadata, and tokens would eventually be forgotten. What's wrong with that? That's just another example of good technical progress for the web platform.

@mkhanas
Copy link
Contributor Author

mkhanas commented Nov 7, 2025

These last 2 comments, and those from the previous issue, have no hint of AI.

I’m not a native speaker, so in the first comment I was just trying to politely make my point. When I saw you didn’t like my style, I just dropped it :)

I see no conflict or even issue with effectively saying "these are two worthy options. Take your pick". Most would, indeed, eventually pick fetch metadata, and tokens would eventually be forgotten.

I know, and that makes perfect sense. I was just trying to play it safe and leave room for discussion, something like:
“If your app is intended to be used only on newer versions (provide versions that support Fetch Metadata headers), go with Fetch Metadata. If for some reason you still need to support older versions (shame on you), use CSRF tokens.”

Let’s do it this way: I’ll fix the language according to the suggestions in your first message, add the missing bits about HSTS, keep the Metadata section at the beginning (so we don’t have to open another PR to move it later), and then ask for a review from the rest — especially the Go folks, where this all started.

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