diff --git a/app/config.go b/app/config.go index c691c60..6a1b297 100644 --- a/app/config.go +++ b/app/config.go @@ -119,3 +119,21 @@ func (a *ConfigVariables) ToConsole(w *ansiterm.TabWriter) { printRow(w, configVar.Name, configVar.Value) } } + +// ToConsoleMasked prints the config vars to the console via the TabWriter, +// obscuring values that look like secrets (see MaskConfigValue). It returns +// the number of values that were masked. +func (a *ConfigVariables) ToConsoleMasked(w *ansiterm.TabWriter) int { + masked := 0 + + for _, configVar := range *a { + value, wasMasked := MaskConfigValue(configVar.Name, configVar.Value) + if wasMasked { + masked++ + } + + printRow(w, configVar.Name, value) + } + + return masked +} diff --git a/app/config_test.go b/app/config_test.go index b5f5516..35c3d9f 100644 --- a/app/config_test.go +++ b/app/config_test.go @@ -3,6 +3,7 @@ package app_test import ( "bytes" "errors" + "strings" "testing" "github.com/apppackio/apppack/app" @@ -72,7 +73,9 @@ func TestConfigVariablesToConsole(t *testing.T) { expected := []byte("FOO:\t\t\tbar\nLONGERVARIABLEFOO:\tbaz\n") - w.Flush() + if err := w.Flush(); err != nil { + t.Fatal(err) + } actual := out.Bytes() if !bytes.Equal(actual, expected) { @@ -80,6 +83,74 @@ func TestConfigVariablesToConsole(t *testing.T) { } } +func TestConfigVariablesToConsoleMasked(t *testing.T) { + t.Parallel() + + c := app.NewConfigVariables([]ssmtypes.Parameter{ + {Name: aws.String("/apppack/apps/myapp/config/SECRET_KEY"), Value: aws.String("example-token-abcdefghijklmnop")}, + {Name: aws.String("/apppack/apps/myapp/config/DATABASE_URL"), Value: aws.String("postgres://user:pw@host:5432/db")}, + {Name: aws.String("/apppack/apps/myapp/config/ENVIRONMENT"), Value: aws.String("production")}, + }) + out := &bytes.Buffer{} + w := ansiterm.NewTabWriter(out, 8, 8, 0, '\t', 0) + maskedCount := c.ToConsoleMasked(w) + + if err := w.Flush(); err != nil { + t.Fatal(err) + } + + if maskedCount != 2 { + t.Errorf("expected 2 masked values, got %d", maskedCount) + } + + output := out.String() + if strings.Contains(output, "example-token-abcdefghijklmnop") { + t.Errorf("expected SECRET_KEY value to be masked, got %s", output) + } + + if strings.Contains(output, ":pw@") { + t.Errorf("expected DATABASE_URL password to be masked, got %s", output) + } + + if !strings.Contains(output, "production") { + t.Errorf("expected ENVIRONMENT value to remain unmasked, got %s", output) + } +} + +// TestConfigExportRemainsUnmasked guards against regressions where masking +// leaks into the machine-readable export/JSON paths, which must always +// remain unmasked (config export is the documented backup/restore path). +func TestConfigExportRemainsUnmasked(t *testing.T) { + t.Parallel() + + c := app.NewConfigVariables([]ssmtypes.Parameter{ + {Name: aws.String("/apppack/apps/myapp/config/SECRET_KEY"), Value: aws.String("example-token-abcdefghijklmnop")}, + {Name: aws.String("/apppack/apps/myapp/config/DATABASE_URL"), Value: aws.String("postgres://user:pw@host:5432/db")}, + }) + + js, err := c.ToJSON() + if err != nil { + t.Fatal(err) + } + + if !strings.Contains(js.String(), "example-token-abcdefghijklmnop") { + t.Errorf("expected ToJSON to contain unmasked SECRET_KEY value, got %s", js.String()) + } + + if !strings.Contains(js.String(), "postgres://user:pw@host:5432/db") { + t.Errorf("expected ToJSON to contain unmasked DATABASE_URL value, got %s", js.String()) + } + + jsUnmanaged, err := c.ToJSONUnmanaged() + if err != nil { + t.Fatal(err) + } + + if !strings.Contains(jsUnmanaged.String(), "example-token-abcdefghijklmnop") { + t.Errorf("expected ToJSONUnmanaged to contain unmasked SECRET_KEY value, got %s", jsUnmanaged.String()) + } +} + func TestConfigVariablesTransform(t *testing.T) { t.Parallel() diff --git a/app/mask.go b/app/mask.go new file mode 100644 index 0000000..1cf7180 --- /dev/null +++ b/app/mask.go @@ -0,0 +1,159 @@ +package app + +import ( + "net/url" + "strings" +) + +// sensitiveNameSubstrings is a case-insensitive deny-list of substrings that, +// when found in a config variable name, mark its value as likely sensitive. +// +// This is a best-effort heuristic, not a security control. It will miss +// things (e.g. STRIPE_SK, FOO_PW) and over-mask others (e.g. AUTH_ENABLED). +var sensitiveNameSubstrings = []string{ + "SECRET", + "PASSWORD", + "PASSWD", + "TOKEN", + "API_KEY", + "APIKEY", + "PRIVATE_KEY", + "CREDENTIAL", + "AUTH", + "SALT", + "SIGNATURE", + "ACCESS_KEY", + "SESSION_KEY", + "DSN", +} + +const ( + // maskChar is used to obscure sensitive values. + maskChar = "•" + // maxMaskRunLength caps the number of mask characters printed so the + // mask never reveals the exact length of the original value. + maxMaskRunLength = 12 + // shortValueMaskLength is the fixed-width mask printed for values that + // are too short to safely reveal partial characters. + shortValueMaskLength = 8 + // minMaskableLength is the value length above which we show the first + // and last 2 characters instead of a fully opaque mask. + minMaskableLength = 8 + // partialRevealLength is the number of characters shown from each end + // of a long value. + partialRevealLength = 2 +) + +// IsSensitiveName reports whether name looks like it holds a secret value, +// based on a case-insensitive substring match against a deny-list. +func IsSensitiveName(name string) bool { + upper := strings.ToUpper(name) + for _, substr := range sensitiveNameSubstrings { + if strings.Contains(upper, substr) { + return true + } + } + + return false +} + +// maskRun returns a run of mask characters, capped at maxMaskRunLength so +// the original value's length is never revealed. +func maskRun(length int) string { + if length > maxMaskRunLength { + length = maxMaskRunLength + } + + return strings.Repeat(maskChar, length) +} + +// MaskValue obscures value, retaining the first and last 2 characters for +// values longer than 8 characters. Values of 8 characters or fewer are +// replaced with a fixed-width mask so their length is never leaked. +func MaskValue(value string) string { + if len(value) <= minMaskableLength { + return maskRun(shortValueMaskLength) + } + + prefix := value[:partialRevealLength] + suffix := value[len(value)-partialRevealLength:] + + return prefix + maskRun(len(value)-2*partialRevealLength) + suffix +} + +// MaskURLPassword parses value as a URL and, if it has a scheme and +// userinfo with a password set, returns a copy with only the password +// component masked. The second return value reports whether masking was +// applied. +// +// The replacement is done via string surgery on the original text (rather +// than reassembling via url.URL.String()) so the scheme, username, host, +// port and path are left byte-for-byte intact and the bullet mask +// characters aren't percent-encoded. +func MaskURLPassword(value string) (string, bool) { + u, err := url.Parse(value) + if err != nil || u.Scheme == "" || u.User == nil { + return value, false + } + + password, ok := u.User.Password() + if !ok || password == "" { + return value, false + } + + schemeSepIdx := strings.Index(value, "://") + if schemeSepIdx == -1 { + // no authority component to do targeted surgery on; mask the + // whole value as a fallback. + return MaskValue(value), true + } + + authorityStart := schemeSepIdx + len("://") + + // the authority component ends at the first '/', '?' or '#' after the + // scheme separator, or at the end of the string. + authorityEnd := len(value) + + for i := authorityStart; i < len(value); i++ { + if c := value[i]; c == '/' || c == '?' || c == '#' { + authorityEnd = i + + break + } + } + + relAt := strings.LastIndex(value[authorityStart:authorityEnd], "@") + if relAt == -1 { + return value, false + } + + atIdx := authorityStart + relAt + + relColon := strings.LastIndex(value[authorityStart:atIdx], ":") + if relColon == -1 { + return value, false + } + + colonIdx := authorityStart + relColon + + masked := value[:colonIdx+1] + maskRun(shortValueMaskLength) + value[atIdx:] + + return masked, true +} + +// MaskConfigValue applies best-effort masking to value based on its config +// variable name. It first tries to mask a password embedded in a URL (e.g. +// DATABASE_URL, REDIS_URL), preserving the rest of the URL. Failing that, +// it falls back to masking the entire value if name looks sensitive. The +// second return value reports whether masking was applied. +func MaskConfigValue(name, value string) (string, bool) { + if masked, ok := MaskURLPassword(value); ok { + return masked, true + } + + if IsSensitiveName(name) { + return MaskValue(value), true + } + + return value, false +} diff --git a/app/mask_test.go b/app/mask_test.go new file mode 100644 index 0000000..5802394 --- /dev/null +++ b/app/mask_test.go @@ -0,0 +1,189 @@ +package app_test + +import ( + "strings" + "testing" + "unicode/utf8" + + "github.com/apppackio/apppack/app" + "github.com/stretchr/testify/assert" +) + +func TestIsSensitiveName(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + expected bool + }{ + {"SECRET_KEY", true}, + {"DJANGO_SECRET_KEY", true}, + {"API_TOKEN", true}, + {"PASSWORD", true}, + {"DB_PASSWD", true}, + {"MY_API_KEY", true}, + {"MYAPIKEY", true}, + {"PRIVATE_KEY", true}, + {"AWS_CREDENTIALS", true}, + {"AUTH_ENABLED", true}, // known false positive - documented tradeoff + {"SALT_ROUNDS", true}, + {"WEBHOOK_SIGNATURE", true}, + {"AWS_ACCESS_KEY_ID", true}, + {"SESSION_KEY", true}, + {"DATABASE_DSN", true}, + {"secret_key", true}, // case-insensitive + {"ENVIRONMENT", false}, + {"DEBUG", false}, + {"PORT", false}, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.expected, app.IsSensitiveName(tt.name)) + }) + } +} + +func TestMaskValue(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + value string + }{ + {"empty", ""}, + {"short", "abc"}, + {"exactly8", "12345678"}, + {"long", "example-token-abcdefghijklmnopqrstuvwxyz4f"}, + {"very_long", strings.Repeat("x", 200)}, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + masked := app.MaskValue(tt.value) + + assert.NotEqual(t, tt.value, masked) + assert.Contains(t, masked, "•") + + if len(tt.value) <= 8 { + assert.Equal(t, "••••••••", masked) + } else { + assert.True(t, strings.HasPrefix(masked, tt.value[:2])) + assert.True(t, strings.HasSuffix(masked, tt.value[len(tt.value)-2:])) + // the mask must never reveal the exact original length + assert.LessOrEqual(t, utf8.RuneCountInString(masked), utf8.RuneCountInString(tt.value)) + } + }) + } +} + +func TestMaskURLPassword(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + value string + wantMasked bool + checkFunc func(t *testing.T, masked string) + }{ + { + name: "postgres with password", + value: "postgres://user:pw@host:5432/db", + wantMasked: true, + checkFunc: func(t *testing.T, masked string) { + t.Helper() + assert.Equal(t, "postgres://user:••••••••@host:5432/db", masked) + assert.NotContains(t, masked, ":pw@") + assert.NotContains(t, masked, "%E2") // must not be percent-encoded + assert.Contains(t, masked, "•") + }, + }, + { + name: "redis without userinfo", + value: "redis://host:6379", + wantMasked: false, + }, + { + name: "no scheme", + value: "not-a-url", + wantMasked: false, + }, + { + name: "empty value", + value: "", + wantMasked: false, + }, + { + name: "user without password", + value: "postgres://user@host:5432/db", + wantMasked: false, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + masked, ok := app.MaskURLPassword(tt.value) + assert.Equal(t, tt.wantMasked, ok) + + if !tt.wantMasked { + assert.Equal(t, tt.value, masked) + } else if tt.checkFunc != nil { + tt.checkFunc(t, masked) + } + }) + } +} + +func TestMaskConfigValue(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + varName string + value string + wantMasked bool + }{ + {"secret key masked", "SECRET_KEY", "example-token-abcdefghijklmnop", true}, + {"django secret masked", "DJANGO_SECRET_KEY", "abcdefghijklmnopqrstuvwxyz", true}, + {"api token masked", "API_TOKEN", "abcdefghijklmnop", true}, + {"auth enabled false positive masked", "AUTH_ENABLED", "true", true}, + {"database url only password masked", "DATABASE_URL", "postgres://user:pw@host:5432/db", true}, + {"redis no userinfo untouched", "REDIS_URL", "redis://host:6379", false}, + {"environment untouched", "ENVIRONMENT", "production", false}, + {"debug untouched", "DEBUG", "false", false}, + {"empty value untouched", "ENVIRONMENT", "", false}, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + masked, ok := app.MaskConfigValue(tt.varName, tt.value) + assert.Equal(t, tt.wantMasked, ok) + + if !tt.wantMasked { + assert.Equal(t, tt.value, masked) + } else { + assert.NotEqual(t, tt.value, masked) + } + }) + } + + t.Run("database url preserves host and db name", func(t *testing.T) { + t.Parallel() + + masked, ok := app.MaskConfigValue("DATABASE_URL", "postgres://user:pw@host:5432/db") + assert.True(t, ok) + assert.Contains(t, masked, "host:5432/db") + assert.NotContains(t, masked, "pw") + }) +} diff --git a/cmd/config.go b/cmd/config.go index da62fc6..be15688 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -42,6 +42,8 @@ var configCmd = &cobra.Command{ DisableFlagsInUseLine: true, } +var revealGet bool + // getCmd represents the get command var getCmd = &cobra.Command{ Use: "get ", @@ -59,7 +61,19 @@ var getCmd = &cobra.Command{ }) ui.Spinner.Stop() checkErr(err) - fmt.Println(*resp.Parameter.Value) + + value := *resp.Parameter.Value + + if !revealGet && isatty.IsTerminal(os.Stdout.Fd()) { + if masked, ok := app.MaskConfigValue(args[0], value); ok { + fmt.Println(masked) + fmt.Fprintln(os.Stderr, "value masked — use --reveal to show") + + return + } + } + + fmt.Println(value) }, } @@ -108,6 +122,8 @@ var unsetCmd = &cobra.Command{ }, } +var revealList bool + // configListCmd represents the list command var configListCmd = &cobra.Command{ Use: "list", @@ -133,12 +149,22 @@ var configListCmd = &cobra.Command{ // minwidth, tabwidth, padding, padchar, flags w := ansiterm.NewTabWriter(os.Stdout, 8, 8, 0, '\t', 0) - if isatty.IsTerminal(os.Stdout.Fd()) { + isTerminal := isatty.IsTerminal(os.Stdout.Fd()) + mask := isTerminal && !revealList + + if isTerminal { w.SetColorCapable(true) } ui.PrintHeaderln(AppName + " Config Vars") - configVars.ToConsole(w) + + maskedCount := 0 + if mask { + maskedCount = configVars.ToConsoleMasked(w) + } else { + configVars.ToConsole(w) + } + checkErr(w.Flush()) if a.IsReviewApp() { @@ -148,10 +174,20 @@ var configListCmd = &cobra.Command{ parameters, err := a.GetConfig() checkErr(err) ui.Spinner.Stop() - parameters.ToConsole(w) + + if mask { + maskedCount += parameters.ToConsoleMasked(w) + } else { + parameters.ToConsole(w) + } + ui.PrintHeaderln(a.Name + " Config Vars (inherited)") checkErr(w.Flush()) } + + if maskedCount > 0 { + fmt.Fprintf(os.Stderr, "%d value(s) masked — use --reveal to show\n", maskedCount) + } }, } @@ -239,9 +275,11 @@ func init() { ) configCmd.AddCommand(getCmd) + getCmd.Flags().BoolVar(&revealGet, "reveal", false, "show secret-looking values in plaintext instead of masking them") configCmd.AddCommand(setCmd) configCmd.AddCommand(unsetCmd) configCmd.AddCommand(configListCmd) + configListCmd.Flags().BoolVar(&revealList, "reveal", false, "show secret-looking values in plaintext instead of masking them") configCmd.AddCommand(configExportCmd) configExportCmd.Flags().BoolVar(&includeManagedVars, "all",