Repository: kopia/kopia
Pull request #2756 (2023-02-10 16:22:10)

feat(cli): new storage config input from file when creating a repo
When creating a repo we are currently required to supply storage config secrets as CLI arguments. CLI commands often get logged which can accidentally expose these creds. This PR extends the `from-config` storage flags to be supplied at the time of creation with a dedicated `--token-file` option or `--token-stdin` option that reads from stdin.

```
echo "${kopia_string}" | kopia repository create from-config --token-stdin
```

```
kopia repository create from-config --token-file /path/to/token-file
```

codecov[bot]: # [Codecov](https://codecov.io/gh/kopia/kopia/pull/2756?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) Report
Base: **75.20**% // Head: **75.23**% // Increases project coverage by **`+0.03%`** :tada:
> Coverage data is based on head [(`f5b13a2`)](https://codecov.io/gh/kopia/kopia/pull/2756?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) compared to base [(`7ada5e1`)](https://codecov.io/gh/kopia/kopia/commit/7ada5e1c3bb3106d1d6d7b2d7ceff40c63eb78a8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).
> Patch coverage: 76.19% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #2756      +/-   ##
==========================================
+ Coverage   75.20%   75.23%   +0.03%     
==========================================
  Files         457      457              
  Lines       35868    35879      +11     
==========================================
+ Hits        26974    26995      +21     
+ Misses       6993     6985       -8     
+ Partials     1901     1899       -2     
```


| [Impacted Files](https://codecov.io/gh/kopia/kopia/pull/2756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) | Coverage Δ | |
|---|---|---|
| [cli/command\_repository\_create.go](https://codecov.io/gh/kopia/kopia/pull/2756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-Y2xpL2NvbW1hbmRfcmVwb3NpdG9yeV9jcmVhdGUuZ28=) | `73.60% <ø> (-0.21%)` | :arrow_down: |
| [cli/command\_repository\_connect\_from\_config.go](https://codecov.io/gh/kopia/kopia/pull/2756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-Y2xpL2NvbW1hbmRfcmVwb3NpdG9yeV9jb25uZWN0X2Zyb21fY29uZmlnLmdv) | `63.63% <72.22%> (+19.51%)` | :arrow_up: |
| [repo/token.go](https://codecov.io/gh/kopia/kopia/pull/2756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-cmVwby90b2tlbi5nbw==) | `67.85% <100.00%> (+14.01%)` | :arrow_up: |
| [internal/server/source\_manager.go](https://codecov.io/gh/kopia/kopia/pull/2756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-aW50ZXJuYWwvc2VydmVyL3NvdXJjZV9tYW5hZ2VyLmdv) | `81.42% <0.00%> (+0.76%)` | :arrow_up: |
| [repo/blob/throttling/throttling\_semaphore.go](https://codecov.io/gh/kopia/kopia/pull/2756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-cmVwby9ibG9iL3Rocm90dGxpbmcvdGhyb3R0bGluZ19zZW1hcGhvcmUuZ28=) | `100.00% <0.00%> (+2.94%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/kopia/kopia/pull/2756?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).

julio-lopez: @Shrekster nit: modified the PR description to remove `--password ....` in order to better reflect the desired usage pattern.

Patch:
diff --git a/cli/command_repository_connect_from_config.go b/cli/command_repository_connect_from_config.go
index c6aa147d..b2b2fd1a 100644
--- a/cli/command_repository_connect_from_config.go
+++ b/cli/command_repository_connect_from_config.go
@@ -2,6 +2,8 @@
 
 import (
 	"context"
+	"io"
+	"os"
 
 	"github.com/alecthomas/kingpin/v2"
 	"github.com/pkg/errors"
@@ -13,6 +15,8 @@
 type storageFromConfigFlags struct {
 	connectFromConfigFile  string
 	connectFromConfigToken string
+	connectFromTokenFile   string
+	connectFromTokenStdin  bool
 
 	sps StorageProviderServices
 }
@@ -20,6 +24,8 @@ type storageFromConfigFlags struct {
 func (c *storageFromConfigFlags) Setup(sps StorageProviderServices, cmd *kingpin.CmdClause) {
 	cmd.Flag("file", "Path to the configuration file").StringVar(&c.connectFromConfigFile)
 	cmd.Flag("token", "Configuration token").StringVar(&c.connectFromConfigToken)
+	cmd.Flag("token-file", "Path to the configuration token file").StringVar(&c.connectFromTokenFile)
+	cmd.Flag("token-stdin", "Read configuration token from stdin").BoolVar(&c.connectFromTokenStdin)
 
 	c.sps = sps
 }
@@ -27,19 +33,27 @@ func (c *storageFromConfigFlags) Setup(sps StorageProviderServices, cmd *kingpin
 func (c *storageFromConfigFlags) Connect(ctx context.Context, isCreate bool, formatVersion int) (blob.Storage, error) {
 	_ = formatVersion
 
-	if isCreate {
-		return nil, errors.New("not supported")
-	}
-
-	if c.connectFromConfigFile != "" {
+	if !isCreate && c.connectFromConfigFile != "" {
 		return c.connectToStorageFromConfigFile(ctx)
 	}
 
 	if c.connectFromConfigToken != "" {
-		return c.connectToStorageFromConfigToken(ctx)
+		return c.connectToStorageFromConfigToken(ctx, c.connectFromConfigToken)
+	}
+
+	if c.connectFromTokenFile != "" {
+		return c.connectToStorageFromStorageConfigFile(ctx)
+	}
+
+	if c.connectFromTokenStdin {
+		return c.connectToStorageFromStorageConfigStdin(ctx)
+	}
+
+	if isCreate {
+		return nil, errors.New("one of --token-file, --token-stdin or --token must be provided")
 	}
 
-	return nil, errors.New("either --file or --token must be provided")
+	return nil, errors.New("one of --file, --token-file, --token-stdin or --token must be provided")
 }
 
 func (c *storageFromConfigFlags) connectToStorageFromConfigFile(ctx context.Context) (blob.Storage, error) {
@@ -56,8 +70,8 @@ func (c *storageFromConfigFlags) connectToStorageFromConfigFile(ctx context.Cont
 	return blob.NewStorage(ctx, *cfg.Storage, false)
 }
 
-func (c *storageFromConfigFlags) connectToStorageFromConfigToken(ctx context.Context) (blob.Storage, error) {
-	ci, pass, err := repo.DecodeToken(c.connectFromConfigToken)
+func (c *storageFromConfigFlags) connectToStorageFromConfigToken(ctx context.Context, token string) (blob.Storage, error) {
+	ci, pass, err := repo.DecodeToken(token)
 	if err != nil {
 		return nil, errors.Wrap(err, "invalid token")
 	}
@@ -69,3 +83,21 @@ func (c *storageFromConfigFlags) connectToStorageFromConfigToken(ctx context.Con
 	//nolint:wrapcheck
 	return blob.NewStorage(ctx, ci, false)
 }
+
+func (c *storageFromConfigFlags) connectToStorageFromStorageConfigFile(ctx context.Context) (blob.Storage, error) {
+	tokenData, err := os.ReadFile(c.connectFromTokenFile)
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to open token file")
+	}
+
+	return c.connectToStorageFromConfigToken(ctx, string(tokenData))
+}
+
+func (c *storageFromConfigFlags) connectToStorageFromStorageConfigStdin(ctx context.Context) (blob.Storage, error) {
+	tokenData, err := io.ReadAll(c.sps.stdin())
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to read token from stdin")
+	}
+
+	return c.connectToStorageFromConfigToken(ctx, string(tokenData))
+}
diff --git a/cli/command_repository_create.go b/cli/command_repository_create.go
index c88543fc..77611690 100644
--- a/cli/command_repository_create.go
+++ b/cli/command_repository_create.go
@@ -57,10 +57,6 @@ func (c *commandRepositoryCreate) setup(svc advancedAppServices, parent commandP
 	c.out.setup(svc)
 
 	for _, prov := range svc.storageProviders() {
-		if prov.Name == "from-config" {
-			continue
-		}
-
 		// Set up 'create' subcommand
 		f := prov.NewFlags()
 		cc := cmd.Command(prov.Name, "Create repository in "+prov.Description)
diff --git a/cli/storage_providers.go b/cli/storage_providers.go
index 52a12963..67a72026 100644
--- a/cli/storage_providers.go
+++ b/cli/storage_providers.go
@@ -2,6 +2,7 @@
 
 import (
 	"context"
+	"io"
 
 	"github.com/alecthomas/kingpin/v2"
 
@@ -15,6 +16,7 @@ type StorageProviderServices interface {
 	EnvName(s string) string
 	setPasswordFromToken(pwd string)
 	storageProviders() []StorageProvider
+	stdin() io.Reader
 }
 
 // StorageFlags is implemented by cli storage providers which need to support a
diff --git a/repo/token.go b/repo/token.go
index ed477955..bf3f99a0 100644
--- a/repo/token.go
+++ b/repo/token.go
@@ -18,9 +18,15 @@ type tokenInfo struct {
 // Token returns an opaque token that contains repository connection information
 // and optionally the provided password.
 func (r *directRepository) Token(password string) (string, error) {
+	return EncodeToken(password, r.blobs.ConnectionInfo())
+}
+
+// EncodeToken returns an opaque token that contains the given connection information
+// and optionally the provided password.
+func EncodeToken(password string, ci blob.ConnectionInfo) (string, error) {
 	ti := &tokenInfo{
 		Version:  "1",
-		Storage:  r.blobs.ConnectionInfo(),
+		Storage:  ci,
 		Password: password,
 	}
 


Test patch:
diff --git a/cli/command_repository_create_test.go b/cli/command_repository_create_test.go
new file mode 100644
index 00000000..486b11af
--- /dev/null
+++ b/cli/command_repository_create_test.go
@@ -0,0 +1,63 @@
+package cli_test
+
+import (
+	"os"
+	"path"
+	"strings"
+	"testing"
+
+	"github.com/kopia/kopia/repo"
+	"github.com/kopia/kopia/repo/blob"
+	"github.com/kopia/kopia/repo/blob/filesystem"
+	"github.com/kopia/kopia/tests/testenv"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestRepositoryCreateWithConfigFile(t *testing.T) {
+	env := testenv.NewCLITest(t, nil, testenv.NewInProcRunner(t))
+
+	_, stderr := env.RunAndExpectFailure(t, "repo", "create", "from-config", "--file", path.Join(env.ConfigDir, "does_not_exist.config"))
+	require.Contains(t, stderr, "can't connect to storage: one of --token-file, --token-stdin or --token must be provided")
+
+	_, stderr = env.RunAndExpectFailure(t, "repo", "connect", "from-config")
+	require.Contains(t, stderr, "can't connect to storage: one of --file, --token-file, --token-stdin or --token must be provided")
+
+	_, stderr = env.RunAndExpectFailure(t, "repo", "create", "from-config", "--token", "bad-token")
+	require.Contains(t, stderr, "can't connect to storage: invalid token: unable to decode token")
+
+	storageCfgFName := path.Join(env.ConfigDir, "storage-config.json")
+	ci := blob.ConnectionInfo{
+		Type:   "filesystem",
+		Config: filesystem.Options{Path: env.RepoDir},
+	}
+	token, err := repo.EncodeToken("12345678", ci)
+	require.Nil(t, err)
+
+	// expect failure before writing to file
+	_, stderr = env.RunAndExpectFailure(t, "repo", "create", "from-config", "--token-file", storageCfgFName)
+	require.Contains(t, strings.Join(stderr, "\n"), "can't connect to storage: unable to open token file")
+
+	require.Nil(t, os.WriteFile(storageCfgFName, []byte(token), 0o600))
+
+	defer os.Remove(storageCfgFName) //nolint:errcheck,gosec
+
+	env.RunAndExpectSuccess(t, "repo", "create", "from-config", "--token-file", storageCfgFName)
+}
+
+func TestRepositoryCreateWithConfigFromStdin(t *testing.T) {
+	runner := testenv.NewInProcRunner(t)
+	env := testenv.NewCLITest(t, nil, runner)
+
+	ci := blob.ConnectionInfo{
+		Type:   "filesystem",
+		Config: filesystem.Options{Path: env.RepoDir},
+	}
+	token, err := repo.EncodeToken("12345678", ci)
+	require.Nil(t, err)
+
+	// set stdin
+	runner.SetNextStdin(strings.NewReader(token))
+
+	env.RunAndExpectSuccess(t, "repo", "create", "from-config", "--token-stdin")
+}