Skip to content

feat(security): set X-Content-Type-Options: nosniff by default#37354

Merged
wxiaoguang merged 11 commits intogo-gitea:mainfrom
SAY-5:fix/default-x-content-type-options-37316
Apr 24, 2026
Merged

feat(security): set X-Content-Type-Options: nosniff by default#37354
wxiaoguang merged 11 commits intogo-gitea:mainfrom
SAY-5:fix/default-x-content-type-options-37316

Conversation

@SAY-5
Copy link
Copy Markdown
Contributor

@SAY-5 SAY-5 commented Apr 22, 2026

Summary

Fixes #37316.

Gitea already sends X-Frame-Options: SAMEORIGIN on HTML responses by default but never emits X-Content-Type-Options. Without that header, browsers fall back to content sniffing and can execute user-uploaded files as scripts when the Content-Type is missing or ambiguous. Per every modern hardening guide (OWASP, MDN, Mozilla Observatory), nosniff is the correct default.

Fix

  • Security.XContentTypeOptions setting, defaulting to "nosniff".
  • Emit the header from services/context alongside X-Frame-Options if the configured value is not "unset".
  • Expose X_CONTENT_TYPE_OPTIONS in [security] of app.example.ini.

unset removes the header, matching the existing X_FRAME_OPTIONS escape hatch for operators who want the raw HTTP behaviour.

Test plan

  • go build ./modules/setting/... ./services/context/... clean
  • go vet ./modules/setting/... ./services/context/... clean
  • go test ./modules/setting/... green
  • Manual: hit any Gitea page with curl -I and confirm X-Content-Type-Options: nosniff is in the response by default; set X_CONTENT_TYPE_OPTIONS = unset in [security] and confirm it's absent.

No behaviour change for operators who already set their own response header via a reverse proxy — the nosniff value is a safe no-op when upstream sets the same header.

Gitea already sends X-Frame-Options: SAMEORIGIN on HTML responses but
never emits X-Content-Type-Options. Browsers then fall back to
content sniffing and can execute user-uploaded files as scripts when
the Content-Type is missing or ambiguous - a widely-recommended
security default that go-gitea#37316 requested.

Add Security.XContentTypeOptions defaulting to 'nosniff', emit it
from services/context alongside X-Frame-Options, and expose
X_CONTENT_TYPE_OPTIONS in app.example.ini. 'unset' removes the
header, matching the existing X_FRAME_OPTIONS escape hatch.

Fixes go-gitea#37316

Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 22, 2026
@github-actions github-actions Bot added the docs-update-needed The document needs to be updated synchronously label Apr 22, 2026
Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member

Fixup done in 4ca393d.

  • Clarify that both security headers are for web ui responses only, API does not set them
  • Add a test that validates both

@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 22, 2026
Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 22, 2026

Note that API responses already set this header in https://github.com/go-gitea/gitea/blob/main/routers/api/v1/api.go#L1749, so this change fills the gap for web router responses.

The following endpoints still don't set it after this change:

/api/internal/*
/api/healthz
/captcha/*
/metrics
/robots.txt
/ssh_info

I don't see why not set this header on all responses without exceptions, maybe it should be refactored to really apply globally, so maybe we should move this securityHeaders to a true global middleware.

@silverwind silverwind self-requested a review April 22, 2026 09:08
@GiteaBot GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 22, 2026
Comment thread tests/integration/security_headers_test.go Outdated
@wxiaoguang wxiaoguang marked this pull request as draft April 22, 2026 10:23
@wxiaoguang
Copy link
Copy Markdown
Contributor

I don't see why not set this header on all responses without exceptions

Because your code was only added to "web" middlewares.

@silverwind
Copy link
Copy Markdown
Member

Doing it on web routes is an improvement, no doubt. But such a header ideally wants to be configured on all routes without exceptions.

@wxiaoguang
Copy link
Copy Markdown
Contributor

See ProtocolMiddlewares

@wxiaoguang
Copy link
Copy Markdown
Contributor

Well, Your AI already used that middleware: handlers = append(handlers, public.ViteDevMiddleware)

… handler

Per @wxiaoguang and @silverwind on go-gitea#37354: X-Content-Type-Options
should be applied on *every* response Gitea emits, not just the web
UI and /api/v1. The gaps called out in review — /api/internal/*,
/api/healthz, /captcha/*, /metrics, /robots.txt, /ssh_info — all go
through ProtocolMiddlewares (installed by routers/init.go and
routers/install/routes.go), so that is the right home.

- New routers/common.SecurityHeadersHandler sets
  X-Content-Type-Options (honouring the existing "unset" escape
  hatch on setting.Security.XContentTypeOptions) globally and is
  now registered from ProtocolMiddlewares.
- Remove the duplicate X-Content-Type-Options Set in
  services/context/context.go; the web middleware now only sets
  X-Frame-Options, which remains web-UI-specific.
- Remove the private securityHeaders() helper in
  routers/api/v1/api.go and the m.BeforeRouting(securityHeaders())
  registration: both are subsumed by the global handler.

No behaviour change for the X-Frame-Options header (still web-UI
only) and no change for operators who set X_CONTENT_TYPE_OPTIONS =
unset. `go build ./routers/... ./services/...` + `go test
./routers/common/...` green locally.
@SAY-5
Copy link
Copy Markdown
Contributor Author

SAY-5 commented Apr 22, 2026

Thanks @wxiaoguang — lifted the handler into routers/common.SecurityHeadersHandler and registered it from ProtocolMiddlewares alongside ChiRoutePathHandler / RequestContextHandler / public.ViteDevMiddleware, so X-Content-Type-Options now covers every endpoint (including the /api/internal/*, /captcha/*, /metrics, /robots.txt, /ssh_info gaps @silverwind listed). Removed the now-redundant X-Content-Type-Options Set in services/context/context.go and the private securityHeaders() wrapper in routers/api/v1/api.go. X-Frame-Options stays in the web-UI middleware since it's clickjacking-specific.

go build ./routers/... ./services/... and go test -tags 'sqlite sqlite_unlock_notify' ./routers/common/... green locally.

@wxiaoguang wxiaoguang marked this pull request as ready for review April 23, 2026 00:48
silverwind and others added 2 commits April 24, 2026 03:42
Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

Acceptable. Sending XFO on every request is a bit of a waste because it only works on renderable content like HTML or SVG, but acceptable. The alternative of trying to detect what is renderable would be hacky and worse.

@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 24, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor

@SAY-5 Thank you for your PR, but please discuss the issues by human. https://github.com/go-gitea/gitea/blob/main/CONTRIBUTING.md#ai-contribution-policy

@wxiaoguang wxiaoguang enabled auto-merge (squash) April 24, 2026 10:32
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 24, 2026

It's fine to post AI responses to PRs but those should always be paired with an actual human response too.

@wxiaoguang approve first.

@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 24, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor

@wxiaoguang approve first.

Oops, forgot. Approved.

@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 24, 2026
@silverwind
Copy link
Copy Markdown
Member

Comment thread custom/conf/app.example.ini Outdated
Signed-off-by: silverwind <me@silverwind.io>
Comment thread custom/conf/app.example.ini Outdated
Signed-off-by: silverwind <me@silverwind.io>
Comment thread custom/conf/app.example.ini Outdated
Signed-off-by: silverwind <me@silverwind.io>
@wxiaoguang wxiaoguang merged commit 6826321 into go-gitea:main Apr 24, 2026
26 checks passed
@GiteaBot GiteaBot added this to the 1.27.0 milestone Apr 24, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 24, 2026
silverwind added a commit to TheFox0x7/gitea that referenced this pull request Apr 24, 2026
* origin/main:
  feat(security): set X-Content-Type-Options: nosniff by default (go-gitea#37354)
  Refactor pull request view (1) (go-gitea#37380)
  Improve AGENTS.md (go-gitea#37382)
  Remove dead CSS (go-gitea#37376)
  Add pr-review e2e test and speed up e2e tests (go-gitea#37345)
  Drop Fomantic tab, checkbox and form patches (go-gitea#37377)
silverwind added a commit to mohammad-rj/gitea that referenced this pull request Apr 24, 2026
* origin/main:
  Stabilize e2e logout propagation test (go-gitea#37403)
  refactor: serve site manifest via `/assets/site-manifest.json` endpoint (go-gitea#37405)
  feat(security): set X-Content-Type-Options: nosniff by default (go-gitea#37354)

# Conflicts:
#	tests/e2e/events.test.ts
silverwind added a commit to silverwind/gitea that referenced this pull request Apr 25, 2026
* origin/main: (51 commits)
  Fix color regressions, add `priority` color (go-gitea#37417)
  [skip ci] Updated translations via Crowdin
  Stabilize e2e logout propagation test (go-gitea#37403)
  refactor: serve site manifest via `/assets/site-manifest.json` endpoint (go-gitea#37405)
  feat(security): set X-Content-Type-Options: nosniff by default (go-gitea#37354)
  Refactor pull request view (1) (go-gitea#37380)
  Improve AGENTS.md (go-gitea#37382)
  Remove dead CSS (go-gitea#37376)
  Add pr-review e2e test and speed up e2e tests (go-gitea#37345)
  Drop Fomantic tab, checkbox and form patches (go-gitea#37377)
  fix: dump with default zip type produces uncompressed zip (go-gitea#37401)
  Allow fast-forward-only merge when signed commits are required (go-gitea#37335)
  Introduce `ActionRunAttempt` to represent each execution of a run (go-gitea#37119)
  Move review request functions to a standalone file (go-gitea#37358)
  Fix repo init README EOL (go-gitea#37388)
  Fix org team assignee/reviewer lookups for team member permissions (go-gitea#37365)
  Remove external service dependencies in migration tests (go-gitea#36866)
  Extend issue context popup beyond markdown content (go-gitea#36908)
  fix: commit status reporting (go-gitea#37372)
  Support for Custom URI Schemes in OAuth2 Redirect URIs (go-gitea#37356)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-update-needed The document needs to be updated synchronously 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.

Include X-Content-Type-Options: nosniff in default response headers

4 participants