Skip to content

feat(ENG-15016): let credential-helper launchers keep install-time settings - #511

Merged
cloudsmith-iduffy merged 4 commits into
masterfrom
iduffy/launcher-install-time-settings
Nov 3, 2026
Merged

cloudsmith-iduffy merged 4 commits into
masterfrom
iduffy/launcher-install-time-settings

Conversation

@cloudsmith-iduffy

@cloudsmith-iduffy cloudsmith-iduffy commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Description

A package manager often runs its credential helper from a process that does not have the environment of the shell that ran cloudsmith credential-helper install, for example Docker Desktop, an IDE or a build agent. The flags and CLOUDSMITH_* variables that you gave at install time are then lost. This PR adds the shared code that writes those settings into the launcher. The Docker helper (#512) and the NuGet provider (#494) use it.

  • install keeps every option that has a CLOUDSMITH_* environment variable and got its value from a flag or the environment, also from a flag on a parent command such as cloudsmith -P work credential-helper install …. The list comes from the Click options themselves, so a new option is kept with no extra code. Values from config.ini are not written, because the helper reads that file itself. The keyring settings come from the hidden options that refactor(no-ticket): read the keyring settings through Click #515 adds to main.
  • The API key, the keyring key, the custom headers and the proxy URL are never written, because other users can read the launcher. OIDC tokens are not options, so they are never written either.
  • write_launcher(env=...) writes the settings as export lines, or as set lines in a Windows batch file. They replace the values of the caller, so the helper gives the same result as the install.
  • On Windows, batch_value() refuses a value that a batch file cannot hold: ", %, a line break, or non-ASCII text outside the user profile. A non-ASCII path under the profile becomes %USERPROFILE%. The path of the standalone binary goes through the same check.
  • An installer opts in with KEEPS_INSTALL_TIME_SETTINGS = True. No installer opts in in this PR.
#!/bin/sh
export CLOUDSMITH_WORKSPACE=acme
export CLOUDSMITH_PROFILE=work
exec cloudsmith credential-helper docker "$@"

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Refactoring
  • Other (please describe)

Additional Notes

This code was in #494. I moved it here and replaced its fixed list of settings with the loop over the Click options.

The install tests now remove CLOUDSMITH_* variables from the environment, so a direnv shell does not change their result.

There is no CHANGELOG entry, because nothing changes for users until an installer opts in.

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

No registered installer enables or forwards the captured settings, leaving the feature inactive.

1 open issue
What changed in this PR

Adds infrastructure for credential-helper launchers to retain install-time settings across different runtime environments.

Changes:

  • Captures selected CLI and environment settings.
  • Adds launcher environment exports and Windows value validation.
  • Tests settings capture, path resolution, and Windows encoding safeguards.
File Description
cloudsmith_cli/​credential_helpers/​launchers.py Adds settings normalization and platform-specific environment rendering.
cloudsmith_cli/​cli/​tests/​commands/​test_credential_helper_install.py Tests settings capture and launcher value handling.
cloudsmith_cli/​cli/​commands/​credential_helper/​manage.py Collects settings for opted-in installers.

Review effort: Balanced

@cloudsmith-iduffy
cloudsmith-iduffy force-pushed the iduffy/launcher-install-time-settings branch 4 times, most recently from c2ee2fb to e1a1c2e Compare October 21, 2026 15:34
@cloudsmith-iduffy
cloudsmith-iduffy removed this pull request from stack #513 October 21, 2026 16:30
@cloudsmith-iduffy
cloudsmith-iduffy force-pushed the iduffy/launcher-install-time-settings branch from e1a1c2e to 23abdbd Compare October 21, 2026 16:30
@cloudsmith-iduffy
cloudsmith-iduffy changed the base branch from master to iduffy/keyring-settings-via-click October 21, 2026 16:30
@cloudsmith-iduffy
cloudsmith-iduffy force-pushed the iduffy/launcher-install-time-settings branch from 23abdbd to 6db2380 Compare October 21, 2026 17:11
@cloudsmith-iduffy
cloudsmith-iduffy marked this pull request as ready for review October 21, 2026 22:07
@cloudsmith-iduffy
cloudsmith-iduffy requested a review from a team as a code owner October 21, 2026 22:07
@cloudsmith-iduffy
cloudsmith-iduffy force-pushed the iduffy/launcher-install-time-settings branch from 6db2380 to 1c68857 Compare October 23, 2026 12:11
Base automatically changed from iduffy/keyring-settings-via-click to master October 23, 2026 12:58
@cloudsmith-iduffy
cloudsmith-iduffy force-pushed the iduffy/launcher-install-time-settings branch from 1c68857 to d68e448 Compare October 23, 2026 12:58
@coillteoir coillteoir self-assigned this Oct 27, 2026
@cloudsmith-iduffy
cloudsmith-iduffy force-pushed the iduffy/launcher-install-time-settings branch from d68e448 to 66fb588 Compare October 28, 2026 11:42
cloudsmith-iduffy and others added 2 commits November 2, 2026 13:28
…542)

Do not retry a request when DNS reports that the API host or the proxy
host does not exist. Retry a temporary DNS failure as before.

Write one line to stderr when the CLI waits before a backoff retry.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ttings

A package manager can run its credential helper from an IDE, Docker
Desktop or a build agent that does not have the environment of the
install. This change adds the shared code that writes the install-time
CLOUDSMITH_* settings into a launcher.

- `write_launcher(env=...)` in `launchers.py` writes the settings as
  `export` lines, or as `set` lines in a Windows batch file. The settings
  replace the values of the caller.
- `batch_value()` refuses a value that a batch file cannot hold. A
  non-ASCII path under the user profile becomes `%USERPROFILE%`.
- `install` passes `launcher_env` to an installer that sets
  `KEEPS_INSTALL_TIME_SETTINGS`. It holds each option with a
  CLOUDSMITH_* environment variable whose value came from a flag or the
  environment, also on a parent command, so the keyring settings of
  `main` are kept too. The API key, the custom headers, the keyring key
  and the proxy never go into it.

No installer sets `KEEPS_INSTALL_TIME_SETTINGS` yet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cloudsmith-iduffy
cloudsmith-iduffy force-pushed the iduffy/launcher-install-time-settings branch from 66fb588 to 25fd194 Compare November 2, 2026 13:28
* origin/master:
  fix(ENG-15484): accept --color on click versions before 8.2 (#582)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Saved TLS settings can incorrectly re-enable certificate verification on replay, while the tests remove safeguards that prevent real error reports.

2 open issues
1 resolved since last review

Review effort: Balanced

Keep the error report controls after the install tests clear the
CLOUDSMITH_* variables.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues remain for the shared credential-helper launcher settings infrastructure.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

@cloudsmith-iduffy
cloudsmith-iduffy merged commit 6b24bf5 into master Nov 3, 2026
24 checks passed
@cloudsmith-iduffy
cloudsmith-iduffy deleted the iduffy/launcher-install-time-settings branch November 3, 2026 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants