Skip to content

Health Checks: Add check for imaging HMAC secret key#21991

Merged
AndyButland merged 7 commits intomainfrom
v17/feature/healthcheck-for-imaging-settings
Mar 3, 2026
Merged

Health Checks: Add check for imaging HMAC secret key#21991
AndyButland merged 7 commits intomainfrom
v17/feature/healthcheck-for-imaging-settings

Conversation

@AndyButland
Copy link
Copy Markdown
Contributor

@AndyButland AndyButland commented Mar 3, 2026

Prerequisites

  • I have added steps to test this contribution in the description below

Description

Adds a new Security group health check that warns administrators when Umbraco:CMS:Imaging:HMACSecretKey is not configured.

Without an HMAC key, any caller can craft arbitrary image resize/crop URLs. The health check surfaces this as a Warning (not an Error — the site functions without it, but it is a security risk) alongside a link to the imaging settings documentation.

Testing

Automated

Unit tests have been added in `ImagingHMACSecretKeyCheckTests.

Manual

  1. Run the site locally without Umbraco:CMS:Imaging:HMACSecretKey set.
  2. Navigate to Settings → Health Check → Security.
  3. Confirm "Imaging HMAC Secret Key" appears with a Warning status and a link to the docs.
  4. Add a base64-encoded 32-byte value under Umbraco:CMS:Imaging:HMACSecretKey in appsettings.json and restart.
  5. Re-run the check — confirm it turns Success.

You can generate a sample HMAC secret key from Powershell with:

[Convert]::ToBase64String((1..32 | ForEach-Object { Get-Random -Maximum 256 }))

Copilot AI review requested due to automatic review settings March 3, 2026 11:24
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 a new Security health check to the Umbraco CMS core that warns administrators when Umbraco:CMS:Imaging:HMACSecretKey is not configured, helping surface the risk of unsigned image manipulation URLs.

Changes:

  • Introduces ImagingHMACSecretKeyCheck to report Warning when the HMAC key is missing and Success when configured.
  • Adds a documentation link constant for the check under Constants.HealthChecks.DocumentationLinks.Security.
  • Adds localization keys (en / en-us) and unit tests covering warning/success and the non-executable action behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Umbraco.Core/HealthChecks/Checks/Security/ImagingHMACSecretKeyCheck.cs Implements the new security health check logic and documentation link behavior.
src/Umbraco.Core/Constants-HealthChecks.cs Adds a documentation URL constant for the new health check.
src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml Adds localized success/warning messages for the new check (en-us).
src/Umbraco.Core/EmbeddedResources/Lang/en.xml Adds localized success/warning messages for the new check (en).
tests/Umbraco.Tests.UnitTests/Umbraco.Core/HealthChecks/ImagingHMACSecretKeyCheckTests.cs Adds unit tests for warning, success, and ExecuteAction throwing.

Copy link
Copy Markdown
Contributor

@nikolajlauridsen nikolajlauridsen left a comment

Choose a reason for hiding this comment

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

Looks good, tests good.

I'll go ahead and approve since everything works, and this is really just a thought.

But in #21976 you create a HmacSecretKeyService which has the method HasHmacSecretKey. Wouldn't it be better to use that rather than duplicate the logic here? Additionally, it begs the question: Shouldn't the same service be used to retrieve the HmacSecretKey as well? Anyways, that's a smaller architectural detail, so I'll still approve this PR, but I won't merge for now so you have a chance to see what you think 😄

@AndyButland
Copy link
Copy Markdown
Contributor Author

Docs PR is here: umbraco/UmbracoDocs#7864

@AndyButland AndyButland changed the title Security: Add health check for Imaging HMAC secret key Health Checks: Add check for Imaging HMAC secret key Mar 3, 2026
@AndyButland AndyButland changed the title Health Checks: Add check for Imaging HMAC secret key Health Checks: Add check for imaging HMAC secret key Mar 3, 2026
@AndyButland
Copy link
Copy Markdown
Contributor Author

It's a good point. I'll update to use the service in the health check. Currently the IHmacSecretKeyService doesn't allow you to retrieve the key, rather you retrieve it from IOptions, which I think is OK. I've been aligning this with the existing ISiteIdentifierService. So I'll keep the updates in this PR limited to just the health check.

@AndyButland AndyButland enabled auto-merge (squash) March 3, 2026 16:38
@AndyButland AndyButland merged commit c8564f3 into main Mar 3, 2026
27 checks passed
@AndyButland AndyButland deleted the v17/feature/healthcheck-for-imaging-settings branch March 3, 2026 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release/17.3.0 status/needs-docs Requires new or updated documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants