mirror of
https://github.com/pgsty/minio.git
synced 2026-09-13 05:54:04 +03:00
fix: harden config environment file parsing
Trim whitespace around assignments, preserve whitespace inside matching quotes, validate portable variable names, and report redacted file-and-line diagnostics. Check config-file Setenv failures instead of silently ignoring invalid entries.\n\nFixes #65
This commit is contained in:
Feng Ruohang
+48
-8
@@ -36,6 +36,7 @@ import (
|
||||
"strings"
|
||||
"syscall"
|
||||
"time"
|
||||
"unicode"
|
||||
|
||||
"github.com/dustin/go-humanize"
|
||||
fcolor "github.com/fatih/color"
|
||||
@@ -540,6 +541,34 @@ func (e envKV) String() string {
|
||||
return fmt.Sprintf("%s=%s", e.Key, e.Value)
|
||||
}
|
||||
|
||||
func isValidEnvName(name string) bool {
|
||||
if name == "" || !isEnvNameStart(name[0]) {
|
||||
return false
|
||||
}
|
||||
for i := 1; i < len(name); i++ {
|
||||
if !isEnvNameStart(name[i]) && (name[i] < '0' || name[i] > '9') {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func isEnvNameStart(ch byte) bool {
|
||||
return ch == '_' || ch >= 'A' && ch <= 'Z' || ch >= 'a' && ch <= 'z'
|
||||
}
|
||||
|
||||
func trimExportPrefix(envEntry string) string {
|
||||
rest, ok := strings.CutPrefix(envEntry, "export")
|
||||
if !ok || rest == "" {
|
||||
return envEntry
|
||||
}
|
||||
trimmed := strings.TrimLeftFunc(rest, unicode.IsSpace)
|
||||
if len(trimmed) == len(rest) {
|
||||
return envEntry
|
||||
}
|
||||
return trimmed
|
||||
}
|
||||
|
||||
func parsEnvEntry(envEntry string) (envKV, error) {
|
||||
envEntry = strings.TrimSpace(envEntry)
|
||||
if envEntry == "" {
|
||||
@@ -554,13 +583,19 @@ func parsEnvEntry(envEntry string) (envKV, error) {
|
||||
Skip: true,
|
||||
}, nil
|
||||
}
|
||||
envTokens := strings.SplitN(strings.TrimSpace(strings.TrimPrefix(envEntry, "export")), config.EnvSeparator, 2)
|
||||
envTokens := strings.SplitN(trimExportPrefix(envEntry), config.EnvSeparator, 2)
|
||||
if len(envTokens) != 2 {
|
||||
return envKV{}, fmt.Errorf("envEntry malformed; %s, expected to be of form 'KEY=value'", envEntry)
|
||||
return envKV{}, errors.New("missing '='")
|
||||
}
|
||||
|
||||
key := envTokens[0]
|
||||
val := envTokens[1]
|
||||
key := strings.TrimSpace(envTokens[0])
|
||||
val := strings.TrimSpace(envTokens[1])
|
||||
if !isValidEnvName(key) {
|
||||
return envKV{}, fmt.Errorf("invalid environment variable name %q", key)
|
||||
}
|
||||
if strings.IndexByte(val, 0) >= 0 {
|
||||
return envKV{}, errors.New("environment variable value contains NUL")
|
||||
}
|
||||
|
||||
// Remove quotes from the value if found
|
||||
if len(val) >= 2 {
|
||||
@@ -587,10 +622,12 @@ func minioEnvironFromFile(envConfigFile string) ([]envKV, error) {
|
||||
defer f.Close()
|
||||
var ekvs []envKV
|
||||
scanner := bufio.NewScanner(f)
|
||||
lineNo := 0
|
||||
for scanner.Scan() {
|
||||
lineNo++
|
||||
ekv, err := parsEnvEntry(scanner.Text())
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, fmt.Errorf("%s:%d: %w", envConfigFile, lineNo, err)
|
||||
}
|
||||
if ekv.Skip {
|
||||
// Skips empty lines
|
||||
@@ -599,7 +636,7 @@ func minioEnvironFromFile(envConfigFile string) ([]envKV, error) {
|
||||
ekvs = append(ekvs, ekv)
|
||||
}
|
||||
if err = scanner.Err(); err != nil {
|
||||
return nil, err
|
||||
return nil, fmt.Errorf("%s: %w", envConfigFile, err)
|
||||
}
|
||||
return ekvs, nil
|
||||
}
|
||||
@@ -666,12 +703,15 @@ func loadEnvVarsFromFiles() {
|
||||
}
|
||||
|
||||
if env.IsSet(config.EnvConfigEnvFile) {
|
||||
ekvs, err := minioEnvironFromFile(env.Get(config.EnvConfigEnvFile, ""))
|
||||
envConfigFile := env.Get(config.EnvConfigEnvFile, "")
|
||||
ekvs, err := minioEnvironFromFile(envConfigFile)
|
||||
if err != nil && !os.IsNotExist(err) {
|
||||
logger.Fatal(err, "Unable to read the config environment file")
|
||||
}
|
||||
for _, ekv := range ekvs {
|
||||
os.Setenv(ekv.Key, ekv.Value)
|
||||
if err := os.Setenv(ekv.Key, ekv.Value); err != nil {
|
||||
logger.Fatal(err, "Unable to set %s from config environment file %s", ekv.Key, envConfigFile)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,8 +19,10 @@ package cmd
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
@@ -181,3 +183,162 @@ MINIO_ROOT_PASSWORD=minio123`,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func Test_minioEnvironFromFileWhitespaceAndValidation(t *testing.T) {
|
||||
testCases := []struct {
|
||||
name string
|
||||
content string
|
||||
want []envKV
|
||||
errLine int
|
||||
errContains string
|
||||
errExcludes string
|
||||
}{
|
||||
{
|
||||
name: "spaces and tabs around separator",
|
||||
content: "MINIO_ROOT_USER = minio\nMINIO_ROOT_PASSWORD\t=\tminio123",
|
||||
want: []envKV{
|
||||
{Key: "MINIO_ROOT_USER", Value: "minio"},
|
||||
{Key: "MINIO_ROOT_PASSWORD", Value: "minio123"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "export tab and quoted spaces",
|
||||
content: "export\tMINIO_ROOT_USER = \" minio user \"\nexport MINIO_ROOT_PASSWORD = ' minio secret '",
|
||||
want: []envKV{
|
||||
{Key: "MINIO_ROOT_USER", Value: " minio user "},
|
||||
{Key: "MINIO_ROOT_PASSWORD", Value: " minio secret "},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "export Unicode whitespace",
|
||||
content: "export\u00a0MINIO_ROOT_USER=value",
|
||||
want: []envKV{
|
||||
{Key: "MINIO_ROOT_USER", Value: "value"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "export is only a standalone prefix",
|
||||
content: "export=value\nexportFOO=bar",
|
||||
want: []envKV{
|
||||
{Key: "export", Value: "value"},
|
||||
{Key: "exportFOO", Value: "bar"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "unquoted whitespace empty value and additional separators",
|
||||
content: "UNQUOTED = value \nEMPTY =\nTOKEN = scheme://user:password@example.com?a=b",
|
||||
want: []envKV{
|
||||
{Key: "UNQUOTED", Value: "value"},
|
||||
{Key: "EMPTY", Value: ""},
|
||||
{Key: "TOKEN", Value: "scheme://user:password@example.com?a=b"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "valid underscore and digits",
|
||||
content: "_MINIO_2=value",
|
||||
want: []envKV{
|
||||
{Key: "_MINIO_2", Value: "value"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "missing separator redacts the line",
|
||||
content: "MINIO_ROOT_PASSWORD=valid\nsuper-secret-without-equals",
|
||||
errLine: 2,
|
||||
errContains: "missing '='",
|
||||
errExcludes: "super-secret-without-equals",
|
||||
},
|
||||
{
|
||||
name: "empty name",
|
||||
content: "=empty-name-secret",
|
||||
errLine: 1,
|
||||
errContains: `invalid environment variable name ""`,
|
||||
errExcludes: "empty-name-secret",
|
||||
},
|
||||
{
|
||||
name: "digit leading name",
|
||||
content: "1MINIO_ROOT_USER=digit-leading-secret",
|
||||
errLine: 1,
|
||||
errContains: `invalid environment variable name "1MINIO_ROOT_USER"`,
|
||||
errExcludes: "digit-leading-secret",
|
||||
},
|
||||
{
|
||||
name: "hyphenated name",
|
||||
content: "MINIO-ROOT-USER=hyphen-secret",
|
||||
errLine: 1,
|
||||
errContains: `invalid environment variable name "MINIO-ROOT-USER"`,
|
||||
errExcludes: "hyphen-secret",
|
||||
},
|
||||
{
|
||||
name: "whitespace in name",
|
||||
content: "MINIO ROOT USER=whitespace-secret",
|
||||
errLine: 1,
|
||||
errContains: `invalid environment variable name "MINIO ROOT USER"`,
|
||||
errExcludes: "whitespace-secret",
|
||||
},
|
||||
{
|
||||
name: "NUL in name",
|
||||
content: "MINIO\x00ROOT=nul-name-secret",
|
||||
errLine: 1,
|
||||
errContains: "invalid environment variable name",
|
||||
errExcludes: "nul-name-secret",
|
||||
},
|
||||
{
|
||||
name: "NUL in value",
|
||||
content: "MINIO_ROOT_USER=before\x00nul-value-secret",
|
||||
errLine: 1,
|
||||
errContains: "environment variable value contains NUL",
|
||||
errExcludes: "nul-value-secret",
|
||||
},
|
||||
{
|
||||
name: "diagnostic has file and line but no value",
|
||||
content: "MINIO_ROOT_USER=valid\nBAD KEY=super-secret-value",
|
||||
errLine: 2,
|
||||
errContains: `invalid environment variable name "BAD KEY"`,
|
||||
errExcludes: "super-secret-value",
|
||||
},
|
||||
}
|
||||
|
||||
for _, testCase := range testCases {
|
||||
t.Run(testCase.name, func(t *testing.T) {
|
||||
tmpfile, err := os.CreateTemp(t.TempDir(), "testfile")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err = tmpfile.WriteString(testCase.content); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err = tmpfile.Close(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
got, err := minioEnvironFromFile(tmpfile.Name())
|
||||
if testCase.errContains == "" {
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !reflect.DeepEqual(got, testCase.want) {
|
||||
t.Errorf("expected %v, got %v", testCase.want, got)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
if err == nil {
|
||||
t.Fatal("expected an error")
|
||||
}
|
||||
errText := err.Error()
|
||||
location := fmt.Sprintf("%s:%d:", tmpfile.Name(), testCase.errLine)
|
||||
if !strings.Contains(errText, location) {
|
||||
t.Errorf("expected error to contain %q, got %q", location, errText)
|
||||
}
|
||||
if !strings.Contains(errText, testCase.errContains) {
|
||||
t.Errorf("expected error to contain %q, got %q", testCase.errContains, errText)
|
||||
}
|
||||
if testCase.errExcludes != "" && strings.Contains(errText, testCase.errExcludes) {
|
||||
t.Errorf("expected error to redact %q, got %q", testCase.errExcludes, errText)
|
||||
}
|
||||
if got != nil {
|
||||
t.Errorf("expected no entries on parse error, got %v", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user