getporter/porter

Porter params show secret data (secret)

Closed this issue · 9 comments

Describe the bug

porter param show porter-hello shows the param (secret) in output

Name: porter-hello
Created: 2023-07-13
Modified: 2023-07-13

---------------------------------------------
  Name            Local Source  Source Type
---------------------------------------------
  mysql_user      fake-user     secret
  wordpress-user  faker-user    secret

To Reproduce

  • Steps to reproduce the behavior:
  • Create porter quickstart in an empty directory porter create
  • Create params by editing the porter.yaml and uncommenting out
 parameters:
   - name: mysql_user
     type: string
     default: wordpress
  • Create porter params porter params generate
  • porter params show porter-hello
Name: porter-hello
Created: 1 minute ago
Modified: 1 minute ago

-----------------------------------------
  Name        Local Source  Source Type
-----------------------------------------
  mysql_user  fake-user     secret

Expected behavior

Name: porter-hello
Created: 1 minute ago
Modified: 1 minute ago

-----------------------------------------
  Name        Local Source  Source Type
-----------------------------------------
  mysql_user                   secret

## Porter Command and Output

porter v1.0.15

Hey @troy0820 let me work on fixing this

This is in plaintext, this also needs to be done during json/yaml as well.

@troy0820 from the expected output, why do we want to hide the environment variable name from the bundle user? Imo, if they have a bunch of parameters set using env vars they need to have this reference.

We don't. This is showing secret. I've updated the issue showing the source type as secret. In the discussion we want to show everything else except the secret.

We don't. This is showing secret. I've updated the issue showing the source type as secret. In the discussion we want to show everything else except the secret.

Wouldn't the same logic apply. We need to know the secret name that is/was/to be set? What would be the security risk as it is right now?

@AGMETEOR I believe we should, if we set it shouldn't it be able to be retrieved?

func TestPorter_ParametersApply(t *testing.T) {
        t.Run("invalid schemaType", func(t *testing.T) {
                // Make sure that we are validating the display parameter set values, and not just the underlying stored parameter set
                ctx := context.Background()
                p := NewTestPorter(t)
                defer p.Close()

                p.AddTestFile("testdata/parameters/mypset.yaml", "mypset.yaml")
                p.TestConfig.TestContext.EditYaml("mypset.yaml", func(yq *yaml.Editor) error {
                        return yq.SetValue("schemaType", "invalidthing")
                })

                opts := ApplyOptions{
                        Namespace: "altns",
                        File:      "mypset.yaml",
                }
                err := p.ParametersApply(ctx, opts)
                tests.RequireErrorContains(t, err, "invalid schemaType")
        })

        t.Run("valid parameter set", func(t *testing.T) {
                ctx := context.Background()
                p := NewTestPorter(t)
                defer p.Close()

                p.AddTestFile("testdata/parameters/mypset.yaml", "mypset.yaml")
                p.TestConfig.TestContext.EditYaml("mypset.yaml", func(yq *yaml.Editor) error {
                        return yq.DeleteNode("namespace")
                })

                opts := ApplyOptions{
                        Namespace: "altns", // Import into this namespace since one isn't set in the file (we removed it above)
                        File:      "mypset.yaml",
                }
                err := p.ParametersApply(ctx, opts)
                require.NoError(t, err, "ParametersApply failed")

                ps, err := p.Parameters.GetParameterSet(ctx, "altns", "mypset")
                require.NoError(t, err, "Failed to retrieve applied parameter set")

                assert.Equal(t, "mypset", ps.Name, "unexpected parameter set name")
                require.Len(t, ps.Parameters, 1, "expected 1 parameter in the set")
                assert.Equal(t, "foo", ps.Parameters[0].Name, "expected the foo parameter mapping defined")
                assert.Equal(t, "secret", ps.Parameters[0].Source.Strategy, "expected the foo parameter mapping to come from a secret")
                assert.Equal(t, "foo_secret", ps.Parameters[0].Source.Hint, "expected the foo parameter mapping to use foo_secret")
        })
}

This test proves that it's intended use of showing the secret should be done as commands porter params show <bundleName> -oplaintext/yaml/json all show the secret and value of the secret.

@schristoff Should we close this?

Closing as the name of the secret is not the "value" of the secret.

func TestPorter_ParametersApply(t *testing.T) {
        t.Run("invalid schemaType", func(t *testing.T) {
                // Make sure that we are validating the display parameter set values, and not just the underlying stored parameter set
                ctx := context.Background()
                p := NewTestPorter(t)
                defer p.Close()

                p.AddTestFile("testdata/parameters/mypset.yaml", "mypset.yaml")
                p.TestConfig.TestContext.EditYaml("mypset.yaml", func(yq *yaml.Editor) error {
                        return yq.SetValue("schemaType", "invalidthing")
                })

                opts := ApplyOptions{
                        Namespace: "altns",
                        File:      "mypset.yaml",
                }
                err := p.ParametersApply(ctx, opts)
                tests.RequireErrorContains(t, err, "invalid schemaType")
        })

        t.Run("valid parameter set", func(t *testing.T) {
                ctx := context.Background()
                p := NewTestPorter(t)
                defer p.Close()

                p.AddTestFile("testdata/parameters/mypset.yaml", "mypset.yaml")
                p.TestConfig.TestContext.EditYaml("mypset.yaml", func(yq *yaml.Editor) error {
                        return yq.DeleteNode("namespace")
                })

                opts := ApplyOptions{
                        Namespace: "altns", // Import into this namespace since one isn't set in the file (we removed it above)
                        File:      "mypset.yaml",
                }
                err := p.ParametersApply(ctx, opts)
                require.NoError(t, err, "ParametersApply failed")

                ps, err := p.Parameters.GetParameterSet(ctx, "altns", "mypset")
                require.NoError(t, err, "Failed to retrieve applied parameter set")

                assert.Equal(t, "mypset", ps.Name, "unexpected parameter set name")
                require.Len(t, ps.Parameters, 1, "expected 1 parameter in the set")
                assert.Equal(t, "foo", ps.Parameters[0].Name, "expected the foo parameter mapping defined")
                assert.Equal(t, "secret", ps.Parameters[0].Source.Strategy, "expected the foo parameter mapping to come from a secret")
                assert.Equal(t, "foo_secret", ps.Parameters[0].Source.Hint, "expected the foo parameter mapping to use foo_secret")
        })
}

This test proves that it's intended use of showing the secret should be done as commands porter params show <bundleName> -oplaintext/yaml/json all show the secret and value of the secret.

@schristoff Should we close this?

@troy0820 the secret value is not shown. We can close this.