Skip to content

Add test for "fetch redirect", add CSS value validation for external render#37207

Merged
wxiaoguang merged 5 commits intogo-gitea:mainfrom
wxiaoguang:fix-security
Apr 14, 2026
Merged

Add test for "fetch redirect", add CSS value validation for external render#37207
wxiaoguang merged 5 commits intogo-gitea:mainfrom
wxiaoguang:fix-security

Conversation

@wxiaoguang
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang commented Apr 14, 2026

See the new tests and comments

By the way, fix the checkAppUrl message for #37212 (also need to be backported together)

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 14, 2026
@wxiaoguang wxiaoguang changed the title Add Add test for "fetch redirect", add CSS value validation for external render Apr 14, 2026
@wxiaoguang wxiaoguang added the backport/v1.26 This PR should be backported to Gitea 1.26 label Apr 14, 2026
@wxiaoguang wxiaoguang added this to the 1.27.0 milestone Apr 14, 2026
@wxiaoguang wxiaoguang requested a review from Copilot April 14, 2026 03:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds coverage for the fetch-based redirect delegate and introduces validation for the gitea-iframe-bgcolor query parameter used by the external render iframe helper to reduce CSS injection risk.

Changes:

  • Add isValidCssColor and gate iframe background style injection on validation.
  • Add a unit test for CSS color validation.
  • Add Go unit tests for FetchRedirectDelegate and adjust bad-request handling to use http.Error.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
web_src/js/external-render-helper.ts Adds CSS color validation and uses it before injecting background styles
web_src/js/external-render-helper.test.ts Adds unit tests for the new CSS color validator
routers/common/redirect.go Uses http.Error on invalid requests; annotates redirect header logic
routers/common/redirect_test.go Adds tests for allowed/blocked redirect behaviors

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread web_src/js/external-render-helper.ts Outdated
Comment thread web_src/js/external-render-helper.ts Outdated
Comment thread web_src/js/external-render-helper.ts Outdated
Comment thread web_src/js/external-render-helper.test.ts Outdated
Comment thread routers/common/redirect.go Outdated

This comment was marked as resolved.

Comment thread web_src/js/external-render-helper.ts Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 14, 2026
Comment thread web_src/js/external-render-helper.ts Outdated
@silverwind
Copy link
Copy Markdown
Member

Why this color validation? What is the attack vector?

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

Why this color validation? What is the attack vector?

Just in case, the rendered content can be changed by a malicious URL.

Comment thread web_src/js/external-render-helper.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 Apr 14, 2026
@bircni bircni added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 14, 2026
@wxiaoguang wxiaoguang enabled auto-merge (squash) April 14, 2026 12:47
@wxiaoguang wxiaoguang merged commit 699eb41 into go-gitea:main Apr 14, 2026
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 14, 2026
@GiteaBot
Copy link
Copy Markdown
Collaborator

I was unable to create a backport for 1.26. @wxiaoguang, please send one manually. 🍵

go run ./contrib/backport 37207
...  // fix git conflicts if any
go run ./contrib/backport --continue

@GiteaBot GiteaBot added the backport/manual No power to the bots! Create your backport yourself! label Apr 14, 2026
@wxiaoguang wxiaoguang deleted the fix-security branch April 14, 2026 13:19
wxiaoguang added a commit to wxiaoguang/gitea that referenced this pull request Apr 14, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor Author

Backport: Add test for "fetch redirect", add CSS value validation for external render (#37207) #37216

@wxiaoguang wxiaoguang added the backport/done All backports for this PR have been created label Apr 14, 2026
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 17, 2026
* main:
  Replace `dropzone` with `@deltablot/dropzone` (go-gitea#37237)
  Add `ExternalIDClaim` option for OAuth2 OIDC auth source (go-gitea#37229)
  Remove error returns from crypto random helpers and callers (go-gitea#37240)
  Use Content-Security-Policy: script nonce (go-gitea#37232)
  Remove htmx (go-gitea#37224)
  Refactor "htmx" to "fetch action" (go-gitea#37208)
  Fix UI regression (go-gitea#37218)
  Fix corrupted JSON caused by goccy library (go-gitea#37214)
  Add test for "fetch redirect", add CSS value validation for external render (go-gitea#37207)
  Fix incorrect concurrency check (go-gitea#37205)
  refactor: simplify ParseCatFileTreeLine and catBatchParseTreeEntries (go-gitea#37210)
  Update go js py dependencies (go-gitea#37204)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/done All backports for this PR have been created backport/manual No power to the bots! Create your backport yourself! backport/v1.26 This PR should be backported to Gitea 1.26 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants