-
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 | ||
|
|
||
| // 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) | ||
|
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. let me know if you would prefer to extract constants for reused values like "password.txt" or 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) | ||
|
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 an odd test case but I believe the correct behavior |
||
| 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.
It is a best practice for sensitive types like
Secretto implement thefmt.Stringerinterface. This prevents the actual secret value from being accidentally exposed in logs if the configuration struct or theSecretvalue itself is printed using%vor%+v.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.
This is a deliberate design decision made because: