Skip to content

Apache will set strict/HSTS for rails in production#23742

Merged
Fryguy merged 1 commit intoManageIQ:masterfrom
jrafanie:fix-hsts-strict-multiple-entries
Mar 5, 2026
Merged

Apache will set strict/HSTS for rails in production#23742
Fryguy merged 1 commit intoManageIQ:masterfrom
jrafanie:fix-hsts-strict-multiple-entries

Conversation

@jrafanie
Copy link
Copy Markdown
Member

@jrafanie jrafanie commented Mar 5, 2026

This avoids setting the same header value twice.

CP4AIOPS-25657

See:
ManageIQ/manageiq-appliance#402
ManageIQ/manageiq-pods#1347

@jrafanie jrafanie added the bug label Mar 5, 2026
@jrafanie
Copy link
Copy Markdown
Member Author

jrafanie commented Mar 5, 2026

Before:

image

@miq-bot miq-bot added the wip label Mar 5, 2026
@jrafanie
Copy link
Copy Markdown
Member Author

jrafanie commented Mar 5, 2026

After:

image

@jrafanie jrafanie removed the wip label Mar 5, 2026
@jrafanie jrafanie changed the title [WIP] Apache will set strict/HSTS for rails in production Apache will set strict/HSTS for rails in production Mar 5, 2026
@miq-bot
Copy link
Copy Markdown
Member

miq-bot commented Mar 5, 2026

Checked commit jrafanie@1833566 with ruby 3.3.10, rubocop 1.56.3, haml-lint 0.69.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. ⭐

config.hsts = "max-age=#{20.years.to_i}"
# Only set HSTS in development/test where Apache isn't fronting Rails
# In production, Apache sets HSTS (see manageiq-https-application.conf line 15)
config.hsts = Rails.env.production? ? SecureHeaders::OPT_OUT : "max-age=#{20.years.to_i}"
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'm fine merging this for now because production is when we have Apache but I wonder if it's better to check something else that says that Apache is fronting it. Off hand I'm not sure what that is.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Note, I wonder if we need to do the same thing with other options here to make sure they're not duplicated but let's tackle that later. I agree with your thinking.

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 thought we had something somewhere to say whether or not to serve static assets. Maybe that was something else

@Fryguy
Copy link
Copy Markdown
Member

Fryguy commented Mar 5, 2026

Curious that the max age number was different. I thought you had the same number on all the places

@jrafanie jrafanie closed this Mar 5, 2026
@jrafanie jrafanie reopened this Mar 5, 2026
@jrafanie
Copy link
Copy Markdown
Member Author

jrafanie commented Mar 5, 2026

@Fryguy I'm not sure what's going on with the security test

@jrafanie
Copy link
Copy Markdown
Member Author

jrafanie commented Mar 5, 2026

The job was not acquired by Runner of type hosted even after multiple attempts

too much load maybe?

@jrafanie
Copy link
Copy Markdown
Member Author

jrafanie commented Mar 5, 2026

ok, 💚 now @Fryguy

@Fryguy Fryguy merged commit ba6e560 into ManageIQ:master Mar 5, 2026
11 of 12 checks passed
@jrafanie jrafanie deleted the fix-hsts-strict-multiple-entries branch March 5, 2026 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants