Skip to content

Commit e39202e

Browse files
cgoetz-inovexj1n-o9r
authored andcommitted
feat(secret flag) add reusable secret flag implementation, update guide (#1366)
* refac(io) set IO once in main to allow overriding with in memory io in tests `print.Printer` had a reference to a `cobra.Command` for using its IO streams. Each Command also used a Printer, resulting in an awkward circular dependency. Refactored Printer to use IO streams directly. When using the application these are set in `main`, when used in tests these can be set to `bytes.Buffer`s. Also replaced usages of `os.Args` with just a string slice. Also set in `main`. `cobra.Command`s `Args` and IO-streams are set in `NewRootCmd` with `traverseCommands`. `CmdParams` also has an `fs.FS`, currently unused but will allow using the real FS during regular use and an in-memory-FS during tests. This change prepares the application for integrative testing while keeping good isolation. Generally speaking the goal is to move all things with side effects into main (compare with https://grafana.com/blog/how-i-write-http-services-in-go-after-13-years/#func-main-only-calls-run) * fix(fmt) run formatter * fix(test) adapt cli args after changing arg slicing * feat(secret flag) add reusable secret flag implementation, update guide Had to split implementation into two parts: `Set()` and `SecretFlagToSP`. Set is only called when an argument is specified on the command line. So moving the `PromptForPassword` logic into `Set` does not work. I'm not sure if the current solution with a specialized `*ToStringPointer` func is the way to go. So I've only refactored `obs-credentials add` as an example. * fix(docs) generate docs * fix(lint) fix linting errors * fix(secretflag) use title case when printing flag usage * fix(fmt) run formatter
1 parent 94c61ab commit e39202e

6 files changed

Lines changed: 267 additions & 10 deletions

File tree

.github/docs/contribution-guide/cmd.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,17 @@ import (
2222

2323
// Define consts for command flags
2424
const (
25-
someArg = "MY_ARG"
26-
someFlag = "my-flag"
25+
someArg = "MY_ARG"
26+
someFlag = "my-flag"
27+
secretFlag = "secret"
2728
)
2829

2930
// Struct to model user input (arguments and/or flags)
3031
type inputModel struct {
3132
*globalflags.GlobalFlagModel
3233
MyArg string
3334
MyFlag *string
35+
Secret *string
3436
}
3537

3638
// "bar" command constructor
@@ -85,8 +87,10 @@ func NewCmd(params *types.CmdParams) *cobra.Command {
8587
}
8688

8789
// Configure command flags (type, default value, and description)
88-
func configureFlags(cmd *cobra.Command) {
90+
func configureFlags(cmd *cobra.Command, params *types.CmdParams) {
8991
cmd.Flags().StringP(someFlag, "shorthand", "defaultValue", "My flag description")
92+
secret := flags.SecretFlag(secretFlag, params)
93+
cmd.Flags().Var(secret, secretFlag, secret.Usage())
9094
}
9195

9296
// Parse user input (arguments and/or flags)
@@ -102,6 +106,7 @@ func parseInput(p *print.Printer, cmd *cobra.Command, inputArgs []string) (*inpu
102106
GlobalFlagModel: globalFlags,
103107
MyArg: myArg,
104108
MyFlag: flags.FlagToStringPointer(p, cmd, someFlag),
109+
Secret: flags.SecretFlagToStringPointer(p, cmd, secretFlag),
105110
}
106111

107112
// Write the input model to the debug logs

CONTRIBUTION.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,13 @@ For prints that are specific to a certain log level, you can use the methods def
7676

7777
For command outputs that should always be displayed, no matter the defined verbosity, you should use the `print` methods `Outputf` and `Outputln`. These should only be used for the actual output of the commands, which can usually be described by "I ran the command to see _this_".
7878

79+
#### Handling secrets
80+
81+
If your command needs secrets as input, please make sure to use `flags.SecretFlag()` and `flags.SecretFlagToStringPointer()`.
82+
These functions implement reading from stdin or a file.
83+
84+
They also support reading the secret value as a command line argument (deprecated, marked for removal in Oct 2026).
85+
7986
### Onboarding a new STACKIT service
8087

8188
If you want to add a command that uses a STACKIT service `foo` that was not yet used by the CLI, you will first need to implement a few extra steps to configure the new service:

docs/stackit_beta_alb_observability-credentials_add.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,15 @@ stackit beta alb observability-credentials add [flags]
1414

1515
```
1616
Add observability credentials to a load balancer with username "xxx" and display name "yyy", providing the path to a file with the password as flag
17-
$ stackit beta alb observability-credentials add --username xxx --password @./password.txt --display-name yyy
17+
$ stackit beta alb observability-credentials add --username xxx --password @./password.txt --displayname yyy
1818
```
1919

2020
### Options
2121

2222
```
2323
-d, --displayname string Displayname for the credentials
2424
-h, --help Help for "stackit beta alb observability-credentials add"
25-
--password string Password. Can be a string or a file path, if prefixed with "@" (example: @./password.txt).
25+
--password string Password. Can be a string (deprecated) or a file path, if prefixed with '@' (example: @./secret.txt). Will be read from stdin when empty.
2626
-u, --username string Username for the credentials
2727
```
2828

internal/cmd/beta/alb/observability-credentials/add/add.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func NewCmd(params *types.CmdParams) *cobra.Command {
3939
Example: examples.Build(
4040
examples.NewExample(
4141
`Add observability credentials to a load balancer with username "xxx" and display name "yyy", providing the path to a file with the password as flag`,
42-
"$ stackit beta alb observability-credentials add --username xxx --password @./password.txt --display-name yyy"),
42+
"$ stackit beta alb observability-credentials add --username xxx --password @./password.txt --displayname yyy"),
4343
),
4444
RunE: func(cmd *cobra.Command, args []string) error {
4545
ctx := context.Background()
@@ -71,14 +71,16 @@ func NewCmd(params *types.CmdParams) *cobra.Command {
7171
return outputResult(params.Printer, model.OutputFormat, resp)
7272
},
7373
}
74-
configureFlags(cmd)
74+
configureFlags(cmd, params)
7575
return cmd
7676
}
7777

78-
func configureFlags(cmd *cobra.Command) {
78+
func configureFlags(cmd *cobra.Command, params *types.CmdParams) {
7979
cmd.Flags().StringP(usernameFlag, "u", "", "Username for the credentials")
8080
cmd.Flags().StringP(displaynameFlag, "d", "", "Displayname for the credentials")
81-
cmd.Flags().Var(flags.ReadFromFileFlag(), passwordFlag, `Password. Can be a string or a file path, if prefixed with "@" (example: @./password.txt).`)
81+
82+
password := flags.SecretFlag(passwordFlag, params)
83+
cmd.Flags().Var(password, passwordFlag, password.Usage())
8284

8385
cobra.CheckErr(flags.MarkFlagsRequired(cmd, usernameFlag, displaynameFlag))
8486
}
@@ -90,7 +92,7 @@ func parseInput(p *print.Printer, cmd *cobra.Command, _ []string) (*inputModel,
9092
GlobalFlagModel: globalFlags,
9193
Username: flags.FlagToStringPointer(p, cmd, usernameFlag),
9294
Displayname: flags.FlagToStringPointer(p, cmd, displaynameFlag),
93-
Password: flags.FlagToStringPointer(p, cmd, passwordFlag),
95+
Password: flags.SecretFlagToStringPointer(p, cmd, passwordFlag),
9496
}
9597

9698
p.DebugInputModel(model)

internal/pkg/flags/secret.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
package flags
2+
3+
import (
4+
"fmt"
5+
"io/fs"
6+
"strings"
7+
8+
"github.com/spf13/cobra"
9+
"github.com/spf13/pflag"
10+
"golang.org/x/text/cases"
11+
"golang.org/x/text/language"
12+
13+
"github.com/stackitcloud/stackit-cli/internal/pkg/print"
14+
"github.com/stackitcloud/stackit-cli/internal/pkg/types"
15+
)
16+
17+
type secretFlag struct {
18+
printer *print.Printer
19+
fs fs.FS
20+
value string
21+
name string
22+
}
23+
24+
func SecretFlag(name string, params *types.CmdParams) *secretFlag {
25+
f := &secretFlag{
26+
printer: params.Printer,
27+
fs: params.Fs,
28+
name: name,
29+
}
30+
return f
31+
}
32+
33+
var _ pflag.Value = &secretFlag{}
34+
35+
func (f *secretFlag) String() string {
36+
return f.value
37+
}
38+
39+
func (f *secretFlag) Set(value string) error {
40+
if strings.HasPrefix(value, "@") {
41+
path := strings.Trim(value[1:], `"'`)
42+
bytes, err := fs.ReadFile(f.fs, path)
43+
if err != nil {
44+
return fmt.Errorf("reading secret %s: %w", f.name, err)
45+
}
46+
f.value = string(bytes)
47+
return nil
48+
}
49+
f.printer.Warn("Passing a secret value on the command line is insecure and deprecated. This usage will stop working October 2026.\n")
50+
f.value = value
51+
return nil
52+
}
53+
54+
func (f *secretFlag) Type() string {
55+
return "string"
56+
}
57+
58+
func (f *secretFlag) Usage() string {
59+
name := cases.Title(language.AmericanEnglish).String(f.name)
60+
return fmt.Sprintf("%s. Can be a string (deprecated) or a file path, if prefixed with '@' (example: @./secret.txt). Will be read from stdin when empty.", name)
61+
}
62+
63+
func SecretFlagToStringPointer(p *print.Printer, cmd *cobra.Command, flag string) *string {
64+
value, err := cmd.Flags().GetString(flag)
65+
if err != nil {
66+
p.Debug(print.ErrorLevel, "convert secret flag to string pointer: %v", err)
67+
return nil
68+
}
69+
if value == "" {
70+
input, err := p.PromptForPassword(fmt.Sprintf("enter %s: ", flag))
71+
if err != nil {
72+
p.Debug(print.ErrorLevel, "convert secret flag %q to string pointer: %v", flag, err)
73+
return nil
74+
}
75+
return &input
76+
}
77+
if cmd.Flag(flag).Changed {
78+
return &value
79+
}
80+
return nil
81+
}

internal/pkg/flags/secret_test.go

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
package flags
2+
3+
import (
4+
"fmt"
5+
"io"
6+
"strings"
7+
"testing"
8+
"testing/fstest"
9+
10+
"github.com/spf13/cobra"
11+
12+
"github.com/stackitcloud/stackit-cli/internal/pkg/testparams"
13+
"github.com/stackitcloud/stackit-cli/internal/pkg/utils"
14+
)
15+
16+
type testFile struct {
17+
path, content string
18+
}
19+
20+
func TestSecretFlag(t *testing.T) {
21+
t.Parallel()
22+
tests := []struct {
23+
name string
24+
value string
25+
want *string
26+
file *testFile
27+
stdin string
28+
wantErr bool
29+
wantStdErr string
30+
}{
31+
{
32+
name: "no value: prompts",
33+
value: "",
34+
want: utils.Ptr("from stdin"),
35+
stdin: "from stdin",
36+
},
37+
{
38+
name: "a value: prints deprecation",
39+
value: "a value",
40+
want: utils.Ptr("a value"),
41+
wantStdErr: "Warning: Passing a secret value on the command line is insecure and deprecated. This usage will stop working October 2026.\n",
42+
},
43+
{
44+
name: "from an existing file",
45+
value: "@some-file.txt",
46+
want: utils.Ptr("from file"),
47+
file: &testFile{
48+
path: "some-file.txt",
49+
content: "from file",
50+
},
51+
},
52+
{
53+
name: "from a non-existing file",
54+
value: "@some-file-with-typo.txt",
55+
wantErr: true,
56+
file: &testFile{
57+
path: "some-file.txt",
58+
content: "from file",
59+
},
60+
},
61+
{
62+
name: "from an existing double-quoted file",
63+
value: `@"some-file.txt"`,
64+
want: utils.Ptr("from file"),
65+
file: &testFile{
66+
path: "some-file.txt",
67+
content: "from file",
68+
},
69+
},
70+
{
71+
name: "from an existing single-quoted file",
72+
value: "@'some-file.txt'",
73+
want: utils.Ptr("from file"),
74+
file: &testFile{
75+
path: "some-file.txt",
76+
content: "from file",
77+
},
78+
},
79+
}
80+
81+
for _, tt := range tests {
82+
t.Run(tt.name, func(t *testing.T) {
83+
t.Parallel()
84+
params := testparams.NewTestParams()
85+
if tt.file != nil {
86+
params.Fs = fstest.MapFS{
87+
tt.file.path: &fstest.MapFile{
88+
Data: []byte(tt.file.content),
89+
},
90+
}
91+
}
92+
flag := SecretFlag("test", params.CmdParams)
93+
cmd := cobra.Command{}
94+
cmd.Flags().Var(flag, "test", flag.Usage())
95+
if tt.stdin != "" {
96+
params.In.WriteString(tt.stdin)
97+
params.In.WriteString("\n")
98+
}
99+
100+
if tt.value != "" { // emulate pflag only calling set when flag is specified on the command line
101+
err := cmd.Flags().Set("test", tt.value)
102+
if err != nil && !tt.wantErr {
103+
t.Fatalf("unexpected error: %v", err)
104+
}
105+
if err == nil && tt.wantErr {
106+
t.Fatalf("expected error, got none")
107+
}
108+
}
109+
110+
got := SecretFlagToStringPointer(params.Printer, &cmd, "test")
111+
112+
if got != tt.want && *got != *tt.want {
113+
t.Fatalf("unexpected value: got %q, want %q", *got, *tt.want)
114+
}
115+
if tt.wantStdErr != "" {
116+
message, err := params.Err.ReadString('\n')
117+
if err != nil && err != io.EOF {
118+
t.Fatalf("reading stderr: %v", err)
119+
}
120+
if message != tt.wantStdErr {
121+
t.Fatalf("unexpected stderr: got %q, want %q", message, tt.wantStdErr)
122+
}
123+
}
124+
})
125+
}
126+
}
127+
128+
func TestSecretFlag_Usage(t *testing.T) {
129+
t.Parallel()
130+
tests := []struct {
131+
in string
132+
want string
133+
}{
134+
{
135+
in: "password",
136+
want: "Password",
137+
},
138+
{
139+
in: "Password",
140+
want: "Password",
141+
},
142+
{
143+
in: "",
144+
want: "",
145+
},
146+
{
147+
in: "secret-key",
148+
want: "Secret-Key",
149+
},
150+
}
151+
for _, tt := range tests {
152+
t.Run(fmt.Sprintf("%q -> %q", tt.in, tt.want), func(t *testing.T) {
153+
t.Parallel()
154+
params := testparams.NewTestParams()
155+
flag := SecretFlag(tt.in, params.CmdParams)
156+
got := flag.Usage()
157+
if !strings.HasPrefix(got, tt.want) {
158+
t.Fatalf("unexpected usage: got %q, want %q", got, tt.want)
159+
}
160+
})
161+
}
162+
}

0 commit comments

Comments
 (0)