Skip to content

enhance: move window.config to JSON, improve CSP format - #39236

Merged
bircni merged 5 commits into
go-gitea:mainfrom
silverwind:csp-config-json
Sep 5, 2026
Merged

enhance: move window.config to JSON, improve CSP format#39236
bircni merged 5 commits into
go-gitea:mainfrom
silverwind:csp-config-json

Conversation

@silverwind

@silverwind silverwind commented Sep 4, 2026

Copy link
Copy Markdown
Member
  • move window.config to go-built json which generates and parses faster
  • remove 3 unused template helpers
  • encode CSP with ' instead of ' so Cloudflare parser does not trip on it

Fixes: #39225

When a reverse proxy rewrites the CSP meta tag and emits a malformed nonce,
browsers drop the invalid token and the policy degrades to `script-src *`,
which blocks every inline script. window.config is then undefined and the
whole frontend dies.

A `<script type="application/json">` block is a data block, not executed, so
it is not subject to script-src and survives that degradation.

Assisted-by: Claude Code:Opus 5
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 4, 2026
@github-actions github-actions Bot added the type/enhancement An improvement of existing functionality label Sep 4, 2026

@wxiaoguang wxiaoguang 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.

Completely AI Hallucination

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Sep 4, 2026
@silverwind

silverwind commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

Completely AI Hallucination

No, it is not. It was fully tested, try this:

<head>
<meta http-equiv=content-security-policy content="script-src * 'nonce-abc''
'sha256-vPvJQz887eJZSZgwg8NtCuSg7k61DtotXq1MmbJ8jD8='">
<script nonce=abc>window.config={a:1}</script>
<script>document.write(`config: ${JSON.stringify(window.config)}`)</script>

In Firefox and Chrome, this prints undefined. In Safari it works. Switching to JSON makes it work in all three:

<head>
<meta http-equiv=content-security-policy content="script-src * 'nonce-abc''
'sha256-SHRdxdXcsATTUja2rx3saPBqCTCfmebidvfWleYWYd0='">
<script nonce=abc type=application/json id=gitea-config>{"a":1}</script>
<script>window.config=JSON.parse(document.querySelector("#gitea-config").textContent);document.write(`config:
${JSON.stringify(window.config)}`)</script>

Also, using JSON saves 1ms of browser main thread time on page load.

This is such a clear improvement, I don't know why I even need to justify why JSON is safer than JS.

@wxiaoguang

wxiaoguang commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Completely AI Hallucination

No, it is not. It was fully tested, try this:

Oh, cool. fully tested

image

@silverwind

Copy link
Copy Markdown
Member Author

I don't know what you want to tell me. JSON for data exchange is the right format. JS is not for data exchange.

@wxiaoguang

Copy link
Copy Markdown
Contributor

For your example, when you have "script-src * 'nonce-abc'' 'sha256-vPvJQz887eJZSZgwg8NtCuSg7k61DtotXq1MmbJ8jD8='" , you mean:

  • <script nonce=abc> won't execute
  • only <script> without nonce executes

Right?

@silverwind

Copy link
Copy Markdown
Member Author

Yes, 'nonce-abc'' is the corrupted form that the Cloudflare parser injects and that extra quote causes the nonce value to become invalid in Firefox and Chrome, so that script is never parsed and window.config is never written.

@wxiaoguang

Copy link
Copy Markdown
Contributor

Then read this again

#39236 (comment)

There are so many places using <script nonce>, none of them would work.

How does this PR "fix"? Isn't it completely AI hallucination?

@silverwind

Copy link
Copy Markdown
Member Author

Yes, all script nonce break with this but window.config is by far the most important one. So it is not a complete fix but a major improvement.

@wxiaoguang

Copy link
Copy Markdown
Contributor

So it is just a refactor or chore, it doesn't fix the real problem.

But you said that "Fixes: #39225", "fully tested".

Also, I don't see why the 1ms time-saving can happen (Also, using JSON saves 1ms of browser main thread time on page load). Just cheated by AI again?

@silverwind

silverwind commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

The speed gain depends on the size of the data. With 277 KB of window.config, the saving is 1.4ms, on small pages like repo home it is 15µs.

I'm testing a solution that completely generates the JSON in go now which will make it more safe and faster to generate while removing some template helpers.

@silverwind

silverwind commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

I think the real reason why we break the Cloudflare parser is the way we encode that meta header with those &#39;:

<meta http-equiv="Content-Security-Policy" content="default-src * data: blob:;script-src * &#39;nonce-9420f50aa4e1d2614de25e05548e01dd&#39;;style-src * &#39;unsafe-inline&#39;;">

Likely their parser looks for literal ' only. I'll try to fix.

@wxiaoguang

Copy link
Copy Markdown
Contributor

With 277 KB of window.config, the saving is 1.4ms, on small pages like repo home it is 15µs.

Are you serious ........ which instance has "277 KB of window.config", the site admin is crazy?

@silverwind

silverwind commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

With 277 KB of window.config, the saving is 1.4ms, on small pages like repo home it is 15µs.

Are you serious ........ which instance has "277 KB of window.config", the site admin is crazy?

Open commit 792b4db in any gitea instance and it will be 277kB of window.config because DiffFileTree is in pageData and its size scales with amount of files in the diff. This diff only has 558 changed files, there will be larger ones.

@wxiaoguang

Copy link
Copy Markdown
Contributor

For special pages, the pageData just should not be used.

pageData was introduced to refactor the legacy messy JS code. Actually, the handlers should explicitly render JSON data for frontend case by case. Just need to refactor more legacy code.

@silverwind

silverwind commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

Yes, and looking at DiffFileTree data, it can probably be massively reduced in size, it's very verbose JSON.

Still checking on this ' encoding topic, that will be the real "fix" here.

@wxiaoguang

Copy link
Copy Markdown
Contributor

Still checking on this ' encoding topic, that will be the real "fix" here.

I think we can build the string directly without HTMLFormat or HTMLEscape. Then the single quote problem is fixed.

Assembling the config map in Go lets html/template serialize and escape it in a
single action, so the template no longer hand-writes JSON and three funcmap
helpers that existed only for it can go.

The CSP meta value is built directly rather than through HTMLFormat so its
source quoting stays literal, matching the header form. The policy is composed
of literals and a hex nonce, and a test locks that invariant because the value
is written into the attribute unescaped.

Assisted-by: Claude Code:Opus 5
@silverwind silverwind changed the title enhance: move window.config into a JSON data block enhance: improve CSP formatting, move window.config to JSON Sep 4, 2026
@silverwind

silverwind commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

All done, ' comes out literal now and has test coverage. `The JSON is now built completely in backend which also makes its construction faster. PR description updated.

@silverwind silverwind changed the title enhance: improve CSP formatting, move window.config to JSON enhance: move window.config to JSON, improve CSP format Sep 4, 2026
@wxiaoguang

Copy link
Copy Markdown
Contributor

Well, there are more new problems. window.config might be accessed before it gets initialized.

@silverwind

Copy link
Copy Markdown
Member Author

Well, there are more new problems. window.config might be accessed before it gets initialized.

I don't think any of our code does that, could only be third party code which imho has no business of accessing window.config. Ideally I would like to get rid of this global entirely, and it seems possible now.

@silverwind

Copy link
Copy Markdown
Member Author

If you want I can attempt to refactor away window.config and make that data only accessible via getConfig() or similar, e.g. completely private to our JS code.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Sep 4, 2026
Comment thread web_src/js/modules/errors.ts
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Sep 5, 2026
@bircni bircni added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 5, 2026
@bircni
bircni enabled auto-merge (squash) September 5, 2026 08:34
@bircni
bircni merged commit eb501f6 into go-gitea:main Sep 5, 2026
23 checks passed
@GiteaBot GiteaBot added this to the 28.0.0 milestone Sep 5, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 5, 2026
@wxiaoguang
wxiaoguang deleted the csp-config-json branch September 5, 2026 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Front-end crash after 1.27.3 upgrade: Cloudflare Zaraz/Analytics corrupts CSP nonce syntax

4 participants