-
Notifications
You must be signed in to change notification settings - Fork 128
loopdb: add Secret type for reading password from file #1131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| package loopdb | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "os" | ||
| "strings" | ||
| ) | ||
|
|
||
| // Secret is a string type that can unmarshal values from files when prefixed | ||
| // with '@'. This allows sensitive values like passwords to be stored in files | ||
| // rather than directly in configuration. | ||
| type Secret string | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a best practice for sensitive types like type Secret string
// String implements the fmt.Stringer interface to prevent the secret from
// being leaked in logs.
func (s Secret) String() string {
if s == "" {
return ""
}
return "xxxxxxxx"
}
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a deliberate design decision made because:
|
||
|
|
||
| // UnmarshalFlag implements go-flags Unmarshaler. If value starts with '@', | ||
| // reads from file at that path. Otherwise uses value directly. | ||
| func (s *Secret) UnmarshalFlag(value string) error { | ||
| if strings.HasPrefix(value, "@") { | ||
| filePath := value[1:] | ||
| content, err := os.ReadFile(filePath) | ||
| if err != nil { | ||
| if os.IsNotExist(err) { | ||
| return fmt.Errorf("secret file not found: %s", | ||
| filePath) | ||
| } | ||
| if os.IsPermission(err) { | ||
| return fmt.Errorf("unable to read secret "+ | ||
| "file (permission denied): %s", | ||
| filePath) | ||
| } | ||
|
|
||
| return fmt.Errorf("failed to read secret file %s: %w", | ||
| filePath, err) | ||
| } | ||
| // Trim trailing whitespace (spaces, tabs, newlines) to handle | ||
| // files created on Windows (CRLF) or Unix (LF), and to avoid | ||
| // invisible trailing spaces causing authentication failures. | ||
| *s = Secret(strings.TrimRight(string(content), " \t\r\n")) | ||
|
|
||
| return nil | ||
| } | ||
| *s = Secret(value) | ||
|
|
||
| return nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,197 @@ | ||
| package loopdb | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
|
|
||
| "github.com/jessevdk/go-flags" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // TestSecretUnmarshalFlag tests the Secret type's UnmarshalFlag method. | ||
| func TestSecretUnmarshalFlag(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| t.Run("direct value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| var s Secret | ||
| err := s.UnmarshalFlag("mypassword") | ||
| require.NoError(t, err) | ||
| require.Equal(t, Secret("mypassword"), s) | ||
| }) | ||
|
|
||
| t.Run("empty value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| var s Secret | ||
| err := s.UnmarshalFlag("") | ||
| require.NoError(t, err) | ||
| require.Equal(t, Secret(""), s) | ||
| }) | ||
|
|
||
| t.Run("file reference", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| // Create a temp file with a password. | ||
| tmpDir := t.TempDir() | ||
| passFile := filepath.Join(tmpDir, "password.txt") | ||
| err := os.WriteFile(passFile, []byte("secretpassword"), 0600) | ||
| require.NoError(t, err) | ||
|
|
||
| var s Secret | ||
| err = s.UnmarshalFlag("@" + passFile) | ||
| require.NoError(t, err) | ||
| require.Equal(t, Secret("secretpassword"), s) | ||
| }) | ||
|
|
||
| t.Run("file with trailing newline", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tmpDir := t.TempDir() | ||
| passFile := filepath.Join(tmpDir, "password.txt") | ||
| err := os.WriteFile(passFile, []byte("secretpassword\n"), 0600) | ||
| require.NoError(t, err) | ||
|
|
||
| var s Secret | ||
| err = s.UnmarshalFlag("@" + passFile) | ||
| require.NoError(t, err) | ||
| require.Equal(t, Secret("secretpassword"), s) | ||
| }) | ||
|
|
||
| t.Run("file with CRLF", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tmpDir := t.TempDir() | ||
| passFile := filepath.Join(tmpDir, "password.txt") | ||
| err := os.WriteFile(passFile, []byte("secretpassword\r\n"), 0600) | ||
| require.NoError(t, err) | ||
|
|
||
| var s Secret | ||
| err = s.UnmarshalFlag("@" + passFile) | ||
| require.NoError(t, err) | ||
| require.Equal(t, Secret("secretpassword"), s) | ||
| }) | ||
|
|
||
| t.Run("file with trailing whitespace", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tmpDir := t.TempDir() | ||
| passFile := filepath.Join(tmpDir, "password.txt") | ||
| err := os.WriteFile(passFile, []byte("secretpassword \t\n"), 0600) | ||
| require.NoError(t, err) | ||
|
|
||
| var s Secret | ||
| err = s.UnmarshalFlag("@" + passFile) | ||
| require.NoError(t, err) | ||
| require.Equal(t, Secret("secretpassword"), s) | ||
| }) | ||
|
|
||
| t.Run("empty file", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tmpDir := t.TempDir() | ||
| passFile := filepath.Join(tmpDir, "password.txt") | ||
| err := os.WriteFile(passFile, []byte(""), 0600) | ||
| require.NoError(t, err) | ||
|
|
||
| var s Secret | ||
| err = s.UnmarshalFlag("@" + passFile) | ||
| require.NoError(t, err) | ||
| require.Equal(t, Secret(""), s) | ||
| }) | ||
|
|
||
| t.Run("file with only newlines", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tmpDir := t.TempDir() | ||
| passFile := filepath.Join(tmpDir, "password.txt") | ||
| err := os.WriteFile(passFile, []byte("\n\n\n"), 0600) | ||
| require.NoError(t, err) | ||
|
|
||
| var s Secret | ||
| err = s.UnmarshalFlag("@" + passFile) | ||
| require.NoError(t, err) | ||
| require.Equal(t, Secret(""), s) | ||
| }) | ||
|
|
||
| t.Run("file with newline in middle", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tmpDir := t.TempDir() | ||
| passFile := filepath.Join(tmpDir, "password.txt") | ||
| err := os.WriteFile(passFile, []byte("pass\nword\n"), 0600) | ||
| require.NoError(t, err) | ||
|
|
||
| var s Secret | ||
| err = s.UnmarshalFlag("@" + passFile) | ||
| require.NoError(t, err) | ||
| require.Equal(t, Secret("pass\nword"), s) | ||
| }) | ||
|
|
||
| t.Run("file not found", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| var s Secret | ||
| err := s.UnmarshalFlag("@/nonexistent/path/to/file") | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), "secret file not found") | ||
| require.Contains(t, err.Error(), "/nonexistent/path/to/file") | ||
| }) | ||
|
|
||
| t.Run("at symbol only", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| // Just "@" means read from empty path, which should fail. | ||
| var s Secret | ||
| err := s.UnmarshalFlag("@") | ||
| require.Error(t, err) | ||
| }) | ||
|
|
||
| t.Run("value starting with at but not file ref", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| // A value like "@myemail" would try to read file "myemail". | ||
| // This should fail because that file doesn't exist. | ||
| var s Secret | ||
| err := s.UnmarshalFlag("@myemail") | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), "secret file not found") | ||
| }) | ||
| } | ||
|
|
||
| // TestSecretGoFlagsIntegration tests that Secret works correctly with the | ||
| // go-flags parser. | ||
| func TestSecretGoFlagsIntegration(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| type Config struct { | ||
| Password Secret `long:"password"` | ||
| } | ||
|
|
||
| t.Run("direct value via flags", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| var cfg Config | ||
| parser := flags.NewParser(&cfg, flags.Default) | ||
| _, err := parser.ParseArgs([]string{"--password=directpass"}) | ||
| require.NoError(t, err) | ||
| require.Equal(t, Secret("directpass"), cfg.Password) | ||
| }) | ||
|
|
||
| t.Run("file reference via flags", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tmpDir := t.TempDir() | ||
| passFile := filepath.Join(tmpDir, "password.txt") | ||
| err := os.WriteFile(passFile, []byte("filepass\n"), 0600) | ||
| require.NoError(t, err) | ||
|
|
||
| var cfg Config | ||
| parser := flags.NewParser(&cfg, flags.Default) | ||
| _, err = parser.ParseArgs([]string{"--password=@" + passFile}) | ||
| require.NoError(t, err) | ||
| require.Equal(t, Secret("filepass"), cfg.Password) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
//nolint:gosecdirective was removed in this change. Since the field name contains "Password", it is likely to be flagged by security linters (likegosecG101) as a potential hardcoded credential. It's recommended to keep the suppression to avoid CI failures.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be ignored: The gosec G101 rule looks for hardcoded credentials in string literals. Since Secret is a type (not a hardcoded value), it shouldn't trigger.
Running the linter showed no warnings in this file via
golangci-lint run -v