MINIO_CONFIG_ENV_FILE silently ignores assignments with spaces around =

[Vonng]

Expected Behavior

MINIO_CONFIG_ENV_FILE should accept the same narrow assignment forms that its existing parser already advertises, while treating insignificant whitespace around the first = as formatting:

MINIO_ROOT_USER = minio
export MINIO_ROOT_PASSWORD = "minio secret"

Whitespace outside the assignment should be discarded. Whitespace enclosed by a matching pair of single or double quotes must remain part of the parsed value.

Malformed assignments must stop startup with a diagnostic that identifies the file and line without logging the value.

Current Behavior

parsEnvEntry trims the complete line, splits once on =, and stores both tokens without trimming them individually. KEY = VALUE therefore becomes the environment entry "KEY "=" VALUE".

On Unix, the trailing space does not make os.Setenv fail. The new entry is installed under the wrong name and every exact lookup of KEY misses it. loadEnvVarsFromFiles also discards the os.Setenv result, so there is no error or warning.

This is especially deceptive with a systemd deployment:

1. A cold start may work because systemd parses EnvironmentFile= itself and installs KEY correctly.
2. mc admin service restart executes the server again with the current process environment rather than asking systemd to reread the file.
3. A changed line such as KEY = new installs KEY while the inherited KEY=old remains active.
4. The restart succeeds but continues using the stale value.

Upstream report: minio#21682.

Impact

This is a configuration-integrity and fail-safe defect, not a direct privilege-escalation primitive: modifying the environment file already requires operator-level access. Its consequences can nevertheless be security-relevant because credential, KMS, identity-provider, audit-target, and trusted-proxy changes can remain stale or fall back to a default with no diagnostic.

The Helm chart also exposes this parser through extraSecret and /tmp/silo-config-env/config.env, so the affected path is part of SILO's supported deployment surface.

Root Cause

• parsEnvEntry trims only the complete line, not the key/value tokens.
• Environment-variable names are not syntax-validated.
• minioEnvironFromFile returns parse errors without a file line number.
• The malformed-entry error includes the complete input line and can disclose a password or token.
• loadEnvVarsFromFiles ignores os.Setenv errors.
• The export prefix is removed with strings.TrimPrefix, even when it is not a standalone token.

Proposed Resolution

Parsing contract

1. Continue to ignore blank lines and lines whose first non-whitespace character is #.
2. Recognize export only when followed by assignment-separating whitespace; a valid bare name such as exportFOO must not be rewritten to FOO.
3. Split on the first = only, so values may contain additional = characters.
4. Apply strings.TrimSpace to the key and value tokens.
5. Strip one matching pair of outer single or double quotes after token trimming. This preserves whitespace inside the quotes.
6. Keep empty values valid.

Validation and diagnostics

1. Accept portable shell/systemd-style environment names only: [A-Za-z_][A-Za-z0-9_]*.
2. Do not restrict names to MINIO_; syntactically valid variables used by embedded components or dependencies remain supported.
3. Reject NUL in a value before applying any entry.
4. Wrap parser failures as <path>:<1-based-line>: <reason>.
5. Report the invalid key where useful, but never include the assignment value in an error.
6. Check every os.Setenv result and fail startup if applying a validated entry still fails.
7. Preserve the existing all-or-nothing parse behavior: no entries are applied until the complete file has parsed successfully.

Regression coverage

Add table-driven tests for:

• spaces and tabs on either side of =;
• bare and standalone export assignments;
• exportFOO remaining a bare variable name;
• single- and double-quoted values with leading, trailing, and interior spaces;
• unquoted outer whitespace trimming;
• empty values and values containing additional = characters;
• valid underscore/digit placement;
• empty, digit-leading, hyphenated, and whitespace-containing names;
• NUL in a name or value;
• missing =;
• exact file/line diagnostics and proof that a secret value is not present in the error.

Verification gate:

go test ./cmd -run 'Test_(parsEnvEntry|minioEnvironFromFile)' -count=1
go test ./cmd -count=1
go vet ./cmd
gofmt -l cmd/common-main.go cmd/common-main_test.go
git diff --check

Compatibility and Non-goals

The intended behavior change is narrow: assignments that previously created unusable whitespace-bearing keys become effective, while malformed names fail loudly instead of being silently ignored. A value that intentionally needs leading or trailing whitespace must quote it.

This issue does not claim full systemd EnvironmentFile= compatibility. Backslash continuations, multiline quoted values, escape processing, semicolon comments, and inline comments remain out of scope and should be handled separately.

Variable removal across mc admin service restart is also a separate inherited-environment problem; this parser fix corrects assignments that are present in the file.

Prior Art

• Upstream issue: Key-value tokens from MINIO_CONFIG_ENV_FILE are not space-trimmed minio/minio#21682
• Closed, unreviewed upstream attempt: fix:Remove spaces from the configuration read from MINIO_CONFIG_ENV_FILE minio/minio#21684
• Candidate fork commit that adds token trimming and basic tests, but not validation or diagnostics: veeceey@2984c6f

[Vonng]

Independent plan review — consensus reached

A read-only Claude Code review was run against the current checkout and this issue's proposed resolution. Two initial Fable attempts ended with the provider returning 529 Overloaded and produced no verdict; they are not counted as reviews. A subsequent Claude Code Sonnet/high review completed with GO WITH FIXES and no question requiring owner arbitration.

The reviewer independently confirmed every reported root cause in cmd/common-main.go: token whitespace is retained, export is stripped without a word boundary, quote removal sees the untrimmed value, names/NUL are not validated, malformed-line errors can disclose values, file errors lack line numbers, and the final os.Setenv result is ignored. It also confirmed the test gaps and agreed that full systemd parsing and inherited-variable removal remain separate work.

The implementation plan is amended by consensus as follows:

1. A missing-= error reports only <path>:<line>: missing '='; it must include zero source-line content because the whole line may be an unformatted secret.
2. This is an intentional compatibility tightening: a malformed/non-portable entry that was previously silently inert may now stop startup. That fail-fast behavior is part of the fix and must be called out in the final record.
3. The standalone export boundary uses Unicode whitespace, consistent with the parser's existing strings.TrimSpace behavior. Regression coverage includes export\tKEY=value and export=value, where the latter remains a variable literally named export.
4. NUL validation belongs in the line-scoped parser so the error receives file and line context without exposing the value.
5. The four unrelated os.Setenv calls that load *_FILE secret variables are outside this issue. Only entries sourced from MINIO_CONFIG_ENV_FILE are in scope.

Agreed minimum code scope remains cmd/common-main.go and cmd/common-main_test.go. Execution can proceed without owner arbitration.

[Vonng]

Local implementation and verification record

The consensus plan has been implemented in the current SILO checkout.

Code scope

Only the server-owned parser and its tests were changed:

• cmd/common-main.go
• cmd/common-main_test.go

The implementation now:

• trims key and value tokens around the first =;
• removes a matching outer quote pair only after trimming, preserving whitespace inside quotes;
• recognizes export only as a standalone prefix separated by Unicode whitespace;
• accepts portable ASCII names matching [A-Za-z_][A-Za-z0-9_]* without imposing a MINIO_ allowlist;
• rejects missing separators, invalid names, and NUL-containing values before any entry is applied;
• returns redacted <path>:<line>: <reason> parse errors without including assignment values;
• parses the complete file before entering the application loop;
• checks and fatally reports any os.Setenv failure for a MINIO_CONFIG_ENV_FILE entry;
• leaves the unrelated *_FILE secret loaders, full systemd syntax, and inherited-variable removal behavior unchanged.

Regression coverage includes spaces/tabs, Unicode whitespace after export, standalone versus name-prefixed export, quoted-space preservation, unquoted trimming, empty values, additional = characters, valid and invalid names, key/value NUL, exact line numbers, all-or-nothing parse results, and proof that secrets do not appear in errors.

Current diff summary:

cmd/common-main.go      |  56 ++++++++++++++---
cmd/common-main_test.go | 161 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 209 insertions(+), 8 deletions(-)

Verification

go test ./cmd -run 'Test_(parsEnvEntry|minioEnvironFromFile)' -count=1
ok github.com/minio/minio/cmd 1.126s

go test ./cmd -count=1
ok github.com/minio/minio/cmd 121.218s

go test -race ./cmd -run 'Test_(parsEnvEntry|minioEnvironFromFile)' -count=1
ok github.com/minio/minio/cmd 2.620s

go vet ./cmd
PASS

gofmt -l cmd/common-main.go cmd/common-main_test.go
(no output)

git diff --check
(no output)

Independent review

• Plan review: Claude Code returned GO WITH FIXES; all five clarifications were accepted and recorded in the preceding consensus comment. There was no owner-arbitration question.
• Implementation review: Claude Code independently inspected the actual two-file diff and surrounding call sites, reran formatting/diff/vet checks, and returned GO with no P0, P1, or P2 findings, no required fix, and no missing test.

Delivery state

This is a local, uncommitted implementation. No commit, push, tag, release artifact, container image, documentation-site deployment, or production rollout is claimed. The issue remains open until the code has an intentional commit/push boundary.

The pre-existing concurrent changes in SECURITY.md and docs/security/advisories.md were not touched or included in this fix.

[Vonng]

Commit record

The reviewed #65 fix has now been committed locally as:

6b0998157 fix: harden config environment file parsing

The commit contains only:

• cmd/common-main.go
• cmd/common-main_test.go

Pre-commit focused regression test:

go test ./cmd -run 'Test_(parsEnvEntry|minioEnvironFromFile)' -count=1
ok github.com/minio/minio/cmd 1.362s

The commit message carries Fixes #65, so the issue will close when this commit is intentionally pushed to the repository's default branch. At the time of this record the commit is local only: it has not been pushed, tagged, released, packaged, or deployed.

The unrelated working-tree changes in SECURITY.md and docs/security/advisories.md remain uncommitted and were not included.

[Vonng]

Resolved on main by 6b0998157. MINIO_CONFIG_ENV_FILE parsing now handles whitespace around assignments, preserves whitespace inside matching quotes, validates portable variable names, reports redacted file-and-line errors, parses before applying changes, and checks environment update failures. The latest main CI is green. This fix has not yet shipped in a tagged release. Closing as completed.