Add a docker credential helper to avoid scary security warnings by matt-richardson · Pull Request #1981 · OctopusDeploy/Calamari

matt-richardson · 2026-06-01T02:42:51Z

Summary

In #1542, I implemented a docker credential helper to solve this the scary warning:

Download package mcr.microsoft.com/dotnet/sdk v8.0 directly from Docker Registry v1 'Docker Hub' at https://index.docker.io
WARNING! Your credentials are stored unencrypted in 'octo-docker-configs/config.json'. 
Configure a credential helper to remove this warning. See 
https://docs.docker.com/go/credential-store/ 
Login Succeeded 
docker pull mcr.microsoft.com/dotnet/sdk:8.0 
8.0: Pulling from dotnet/sdk 
Digest: sha256:7fd287cec03e027e39be13fb06ca1347f1338c7a6ab4ad305d94d91852693f70 
Status: Image is up to date for mcr.microsoft.com/dotnet/sdk:8.0 
mcr.microsoft.com/dotnet/sdk:8.0 
Staged mcr.microsoft.com/dotnet/sdk version 8.0 (807.973 MB) from Docker Registry v1 'Docker Hub' at https://index.docker.io.

Previously it was implemented as a Calamari sub command, and ended up with quite a lot of messy changes through the startup logging + plumbing.

Results

This PR approaches it as a standalone, self-contained, trimmed executable docker-credential-octopus that Docker invokes directly.

This all exists behind the feature toggle (calamari-use-docker-credential-helper).

⚠️ Does this change require a corresponding Server Change?

yes, it needs a feature toggle slug added

⚠️ If so - please add a "Requires Server Change" label to this PR!
:done:

…cAttribute.cs

…s/ArtifactoryPackageDownloaderFixture.cs

…adeFixture.cs

…s/HelmChartPackageDownloaderFixture.cs

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…al-helper-refactor # Conflicts: # build/Build.PackageCalamariProjects.cs # source/Calamari.Common/CalamariFlavourProgram.cs # source/Calamari.Common/FeatureToggles/OctopusFeatureToggle.cs

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…project reference The merge duplicated package/project references and reformatted the file. Reset to main's version and re-applied only the Calamari.DockerCredentialHelper reference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ture and Calamari.sln - DockerImagePackageDownloaderFixture.cs: reset to main, re-applied only the new AfterCreatingDockerConfigFile_ShouldRemoveIt test (drops ~106 lines of whitespace churn). - Calamari.sln: reset to main, added only the Calamari.DockerCredentialHelper project entry (fixes missing EndProject and duplicate config entries from the merge). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The previous logic read Octopus.Action.Package.DownloadOnTentacle (a boolean flag) and a non-existent SensitiveVariablesPassword variable, falling back to a constant string — none of which is a real secret, and Server's --variablesPassword isn't exposed via IVariables. The credential helper only needs a password that survives a single acquisition (store during docker login, get during docker pull, both inheriting OCTOPUS_CREDENTIAL_PASSWORD). Generate a per-acquisition AesEncryption.RandomString(16), mirroring how the script bootstrappers protect sensitive variables for a child process. Drops the now-unused IVariables parameter. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…sertions - Build overlay: publish the helper EnableSelfContained() to match Calamari's publish. SetSelfContained(OperatingSystem.IsWindows()) left it framework-dependent on non-Windows build hosts, so the apphost would not use Calamari's bundled runtime and could fail to start on targets without a registered .NET runtime. - Restore the "Cleaned up Docker credential files" verbose log dropped during the earlier trim of DockerCredentialHelper. - Align DockerImagePackageDownloaderCredentialHelperFixture assertions with the actual log strings, and drop the obsolete SensitiveVariablesPassword test setup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

docker login invokes the helper's store operation itself (using the same registry key it later uses for get), so pre-storing the credential was redundant and, for custom registries, used a server-URL key that did not match Docker's. Removing it also drops the now-unused username/password parameters, the DockerCredentialStore field, and its using. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Wrap the login/pull/inspect acquisition in try/finally so the temporary DOCKER_CONFIG directory and credential-helper artifacts (config.json, encrypted .cred files, and the OCTOPUS_CREDENTIAL_PASSWORD/PATH env entries) are removed even when PerformPull or GetImageDetails throws after a successful login. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…d overlay) - DockerCredentialProtocol: handle the 'list' verb (return {}), so the helper conforms to Docker's credential-helper contract for operations beyond store/get/erase. - DockerImagePackageDownloader: move the no-helper login fallback out of the retried delegate into a one-time catch in DownloadPackage, so the retry strategy can't re-enter PerformLogin after the helper was torn down (no more misleading repeated fallback logs). - Build overlay: publish the helper to a separate folder and copy it into Calamari's runtime folder, verifying that any shared file is byte-identical before overwriting and failing the build if a shared dependency has diverged between the two projects. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The byte-identical check failed the Windows build on Calamari.Common.dll: two independent deterministic builds of the same managed assembly differ in their MVID. Compare by file version instead, which still catches a genuinely divergent shared dependency without false-positiving on build-to-build MVID/timestamp differences. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Calamari's assemblies are stamped with the OctoVersion via SetVersion/SetInformationalVersion, but the credential-helper overlay publish wasn't, so its bundled Calamari.Common.dll got the default 1.0.0.0 while Calamari ships 2026.x. The overlay verification correctly caught this: the helper's deps.json would expect a different Calamari.Common version than is present at runtime. Stamp the helper publish with the same version so the shared assembly matches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

… folder The helper depended on Calamari.Common only for AesEncryption, which dragged in Calamari.Common's entire transitive closure (Autofac, NuGet, YamlDotNet, …). Overlaying that into Calamari's folder caused repeated shared-dependency version divergences (Calamari.Common.dll, System.Text.Encodings.Web). Since the helper is now the sole reader/writer of the .cred files (docker login stores, docker pull gets), its crypto only needs to be self-consistent. So: - Drop the Calamari.Common reference; give the helper its own minimal AesEncryption (as Calamari.AzureWebApp.NetCoreShim already does) and move DockerCredentialStore into the helper. - Use a System.Text.Json source-generated context so it stays trim-safe. - Publish self-contained, trimmed, single-file (~14 MB) into its own docker-credential-helper subfolder beside Calamari; the downloader adds that folder to PATH. No shared files, so the overlay + version-verification machinery is removed entirely. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The helper is a single self-contained trimmed binary with a unique name, so it can sit directly in Calamari's folder rather than a subfolder: publish to a staging dir, copy the docker-credential-octopus* file(s) into publish/Calamari/<rid>, and point PATH at the app base directory. This avoids relying on a nested folder surviving consolidation/extraction, and the binary is signed by the existing top-level signing scan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…lish, align System.Text.Json The helper's PublishSingleFile/PublishTrimmed csproj properties leaked into how Calamari.Tests consumed the project reference, producing a non-runnable apphost in the test output (Docker reported "docker-credential-octopus: executable file not found in $PATH"). Move those options to the packaging publish in the Nuke build instead. Also reference System.Text.Json 9.0.16 (matching the rest of the solution) so the framework-dependent apphost's deps.json agrees with the assemblies in the shared test output directory, fixing a System.IO.Pipelines 9.0 load failure. Verified locally: the apphost in the Calamari.Tests publish output runs store/get correctly, and the packaged publish still produces a working ~14 MB trimmed single-file binary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Zipping the package (System.IO.Compression) drops the Unix executable bit, so after the package is unzipped the helper binary is not executable. Docker resolves credential helpers with Go's exec.LookPath, which ignores non-executable files and reports "executable file not found in $PATH" (login then falls back to plaintext). PR #1542 chmod'd its helper script for the same reason; restore it here for the binary. Verified locally that a non-executable file is invisible to PATH lookup and chmod +x fixes it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…back, stronger test - docker-credential-octopus reads/writes stdin/stdout as explicit UTF-8 so non-ASCII credentials aren't mangled on platforms whose console code page isn't UTF-8 (e.g. Windows). - When credential-helper login fails and we fall back to a plain docker login, log a Warning (was Verbose) noting credentials will be stored unencrypted — a silent plaintext fallback defeats the point of the feature. - Strengthen CredentialHelper_EnabledByDefault test to assert the fallback did NOT occur, so it can't pass via the plaintext path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Docker already prints its own "credentials stored unencrypted" warning when the fallback login runs, so an additional Warn from Calamari is redundant. Keep a quiet Verbose diagnostic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Defense-in-depth: although the .cred files only contain ciphertext, restrict them and their directory to the owner on Unix. No-op on Windows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…al-helper-refactor

APErebus

Generally this looks pretty good 👍

No major concerns and the approach is pretty good

- Convert DockerCredential/StoreRequest/GetResponse to init-only records - Trim verbose comments in AesEncryption and DockerCredentialHelper Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…al-helper-refactor

matt-richardson added 30 commits August 28, 2025 14:47

Add a credential helper to avoid plain-text storage warnings

2bb8b15

Improve debugability

4a48588

Make it works

91e8425

Try fixing DockerCredentialScriptsFixture

5a456b9

Update DockerCredentialScriptsFixture.cs

5f7d88b

Merge branch 'main' into mattr/docker-credential-helper

d12b8c6

Update DockerCredentialScriptsFixture.cs

aeb2bf8

Update docker-credential-octopus.ps1

55abbf0

Update DockerCredentialScriptsFixture.cs

2a3111b

Update DockerCredentialScriptsFixture.cs

341a9e4

Update docker-credential-octopus.ps1

ca0e60e

Merge branch 'main' into mattr/docker-credential-helper

2c585bd

Update Build.cs

6f6a96c

Update Build.cs

f9dda25

Update KubernetesContextScriptWrapperLiveFixture.cs

0a64a30

Update InstallTools.cs

d021330

Ignore on CloudMac & AmazonLinux agent

3f555d8

Use vars from password store

70f2b0d

Merge branch 'main' into mattr/docker-credential-helper

af2d579

Fix bad merge

0e5ca6d

Fix bad merge

a233717

Undo changes to RequiresNonMacAttribute

09645a4

Discard changes to source/Calamari.Testing/EnvironmentVariables.cs

ede2e57

Discard changes to source/Calamari.Testing/Requirements/RequiresNonMa…

f5c7961

…cAttribute.cs

Discard changes to source/Calamari.Tests/Fixtures/Integration/Package…

bd67658

…s/ArtifactoryPackageDownloaderFixture.cs

Discard changes to source/Calamari.Testing/EnvironmentVariables.cs

2b617bb

Discard changes to source/Calamari.Tests/KubernetesFixtures/Helm3Upgr…

5998e99

…adeFixture.cs

Discard changes to source/Calamari.Tests/Fixtures/Integration/Package…

e120f7c

…s/HelmChartPackageDownloaderFixture.cs

Merge branch 'main' into mattr/docker-credential-helper

a483bda

undo changes we dont want

a13c75b

matt-richardson and others added 22 commits June 1, 2026 12:57

Remove design spec doc from PR

4ea35a2

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Revert unrelated BOM change to CommandResult.cs

38ae180

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Merge remote-tracking branch 'origin/main' into mattr/docker-credenti…

1ded4df

…al-helper-refactor # Conflicts: # build/Build.PackageCalamariProjects.cs # source/Calamari.Common/CalamariFlavourProgram.cs # source/Calamari.Common/FeatureToggles/OctopusFeatureToggle.cs

Revert unrelated BOM change to PackagedScriptConventionFixture.cs

994bd40

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Include both versions in the overlay divergence error message

74c58fe

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Restrict credential files to the owner (0600 file, 0700 dir)

a83bb87

Defense-in-depth: although the .cred files only contain ciphertext, restrict them and their directory to the owner on Unix. No-op on Windows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Refactor credential-file permission helpers

2f6ffd1

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

matt-richardson added the Requires Server Change label Jun 2, 2026

matt-richardson changed the title ~~Docker credential helper as a standalone app (supersedes #1542)~~ Add a docker credential helper to avoid scary security warnings Jun 2, 2026

matt-richardson added 2 commits June 2, 2026 17:08

Cleanup

b722a66

Merge remote-tracking branch 'origin/main' into mattr/docker-credenti…

44ea463

…al-helper-refactor

APErebus reviewed Jun 2, 2026

View reviewed changes

matt-richardson and others added 3 commits June 3, 2026 11:08

Address PR review: convert credential DTOs to records, trim comments

8830f20

- Convert DockerCredential/StoreRequest/GetResponse to init-only records - Trim verbose comments in AesEncryption and DockerCredentialHelper Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Remove unused param

d1d402d

Merge remote-tracking branch 'origin/main' into mattr/docker-credenti…

aa8cb03

…al-helper-refactor

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a docker credential helper to avoid scary security warnings#1981

Add a docker credential helper to avoid scary security warnings#1981
matt-richardson wants to merge 90 commits into
mainfrom
mattr/docker-credential-helper-refactor

matt-richardson commented Jun 1, 2026 •

edited

Loading

Uh oh!

APErebus left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

matt-richardson commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Results

Uh oh!

APErebus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

matt-richardson commented Jun 1, 2026 •

edited

Loading