Skip to content

feat: allow fetching token from shell command#1405

Open
phm07 wants to merge 2 commits into
mainfrom
token-command
Open

feat: allow fetching token from shell command#1405
phm07 wants to merge 2 commits into
mainfrom
token-command

Conversation

@phm07
Copy link
Copy Markdown
Contributor

@phm07 phm07 commented May 8, 2026

This PR allows the CLI to run a shell command to fetch the HCLOUD_TOKEN for each run, making it possible to use a keyring or password manager to securely store the token.

Fixes #808

@phm07 phm07 self-assigned this May 8, 2026
@phm07 phm07 requested a review from a team as a code owner May 8, 2026 14:05
Comment on lines +39 to +43
if runtime.GOOS == "windows" {
cmd = exec.Command("cmd", "/C", cmdStr)
} else {
cmd = exec.Command("sh", "-c", cmdStr)
}
Copy link
Copy Markdown
Member

@jooola jooola May 12, 2026

Choose a reason for hiding this comment

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

A few questions:

  • How can one define this config globally and pick the right context? See point below.
  • Why not pass down more details about the current context? E.g. as environment variables?
  • Why use a shell at all? I'd leave this for the users to pick a shell if needed
  • Could we store the command as list of strings, instead of a string? This makes passing down the arguments cleaner. Or we implement a similar approach like the docker CMD: If we have a list of string, no shell, if we have a string command, wrap it inside a shell?

Copy link
Copy Markdown
Contributor Author

@phm07 phm07 May 15, 2026

Choose a reason for hiding this comment

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

How can one define this config globally and pick the right context? See point below.
Why not pass down more details about the current context? E.g. as environment variables?

Good point, env variables would fix this

Why use a shell at all? I'd leave this for the users to pick a shell if needed

A shell offers more features like pipes, input/output file redirects etc. Also it allows passing the command as one string instead of a string slice, see below.

Could we store the command as list of strings, instead of a string? This makes passing down the arguments cleaner. Or we implement a similar approach like the docker CMD: If we have a list of string, no shell, if we have a string command, wrap it inside a shell?

List of strings would be fine, except the only problem would be how to configure it using the hcloud context create command. Passing it as a slice would mean passing --token-command multiple times which would be very cumbersome or to separate the parts with commas which would mean you couldn't use a comma in your token command. Accepting both a string and a string slice would be fine but I don't know if that would play well with config parsing.

Comment on lines +44 to +47
out, err := cmd.Output()
if err != nil {
return "", fmt.Errorf("could not retrieve token: %w", err)
}
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.

Should we forward the command stderr to the cli stderr?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Store token in a more secure manner , support pairs of tokens

2 participants