Skip to content

Add HSTS header to static assets - #15470

Merged
szaimen merged 1 commit into
masterfrom
fix/hsts-static-assets
Aug 21, 2026
Merged

Add HSTS header to static assets#15470
szaimen merged 1 commit into
masterfrom
fix/hsts-static-assets

Conversation

@janepie

@janepie janepie commented Aug 20, 2026

Copy link
Copy Markdown
Member

☑️ Resolves

nginx only inherits add_header directives from an outer level if the inner level defines none of its own. Because this block defines its own headers, the server-level Strict-Transport-Security is dropped and has to be added here too

Signed-off-by: janepie <49834966+janepie@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

📖 Documentation Preview

🔍 Open preview →

No RST documentation pages changed in this PR.

Last updated: Thu, 20 Aug 2026 16:25:32 GMT

@szaimen
szaimen requested review from joshtrichards and pabzm August 20, 2026 19:40

@joshtrichards joshtrichards left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I can't think of a better way to do this right now, even with restructuring. When Nginx 1.29.3+ is ubiquitous we can switch to using add_header_inherit merge;, but don't think we're there yet...

That said, I'm not sure it really matters since an HSTS policy is host-level not request path or asset driven. Once the browser gets the policy, it'll apply it to all requests thereafter. Maybe max-age refreshing, but other requests will cover that in practice.

Technically, having it here though would be more consistent and possibly in some weird edge cases it would head-off compliance / heading-scanning noise. But the confusion and noise from being duplicated in the config bugs me unless someone thinks it's really important... ;-)

Good catch, @janepie, in any case!

@pabzm pabzm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, thank you for the catch and the fix!

One idea, which shall not block this from being merged: Maybe we should add a comment to both occurrences of this block to also take care of the other occurrence, if it shall be changed?

(That would consequently also matter for the other repeated options, though.)

@szaimen szaimen added this to the Nextcloud 35 milestone Aug 21, 2026
@szaimen
szaimen merged commit 79d6ca8 into master Aug 21, 2026
25 checks passed
@szaimen
szaimen deleted the fix/hsts-static-assets branch August 21, 2026 09:27
@szaimen

szaimen commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

/backport to dtsble34

@szaimen

szaimen commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

/backport to dtsble33

@szaimen

szaimen commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

/backport to stable33

@szaimen

szaimen commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

/backport to stable34

Comment on lines +229 to +235
# HSTS settings
# WARNING: Only add the preload option once you read about
# the consequences in https://hstspreload.org/. This option
# will add the domain to a hardcoded list that is shipped
# in all major browsers and getting removed from this list
# could take several months.
#add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makes sense, just noting that the indentation is off (2 tabs vs 8 spaces):

Suggested change
# HSTS settings
# WARNING: Only add the preload option once you read about
# the consequences in https://hstspreload.org/. This option
# will add the domain to a hardcoded list that is shipped
# in all major browsers and getting removed from this list
# could take several months.
#add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always;
# HSTS settings
# WARNING: Only add the preload option once you read about
# the consequences in https://hstspreload.org/. This option
# will add the domain to a hardcoded list that is shipped
# in all major browsers and getting removed from this list
# could take several months.
#add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always;

That Nginx skip any parent location headers, once a child location has any header-related directive (add_header Cache-Control here), is pretty annoying, an non-intuitive if not already used to nested Nginx configurations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants