non recognized options are silently ignored

[bam80]

Thus, #514 was hard to deduce.

Expected behavior on unknown option is either:

• abort the app start (preferable)
• or warning in the log, at least

@dolonet

[dolonet]

Confirmed. internal/config/parse.go calls toml.Unmarshal from pelletier/go-toml/v2 without strict mode, so unknown keys are dropped without a trace.

Going with abort rather than warn:

• The library already gives us this for free: toml.NewDecoder(r).DisallowUnknownFields() returns a StrictMissingError with line/column, which is exactly the "hard to deduce" pain point in this issue.
• Warning-only is structurally awkward — the logger is built from *config.Config after Parse() returns (run_proxy.go:302), so we'd have to thread warnings through the return signature and hope users actually read startup logs.
• Backward-compat for renamed keys is already handled by mtg doctor, which is the documented upgrade path. We don't need a second, quieter compatibility layer.
• Behavior aligns with how cobra treats unknown CLI flags: hard error, fix your input.

If anyone has a config that would break under strict parsing, please share it here before I cut the PR — that's the one thing that would push me toward warn-mode or an opt-out flag.

Picking this up.

[dolonet]

Small walk-back on "picking this up" — internal/config/parse.go is one of the areas where I should wait for a maintainer signal before opening a PR, even on a small diff. The proposal above stands as the design; if the strict-abort direction doesn't fit, this is fine to close. Otherwise I'll cut the PR once there's a green light.

[dolonet]

@9seconds — this needs your call before I cut a PR, since it touches internal/config/parse.go. Small diff, but not a one-liner (I under-scoped that in the first draft of this comment — corrected below).

Proposal: switch Parse() to strict TOML decoding so an unknown key aborts startup with a line/column pointer instead of being silently dropped.

The naive version — DisallowUnknownFields() + wrap with %w — does not actually deliver the line/col: StrictMissingError.Error() only returns the generic "strict mode: fields in the document are missing in the target struct". The useful rendering lives in StrictMissingError.String(), so the code has to type-assert for it:

import (
	"bytes"
	"encoding/json"
	"errors" // added
	"fmt"

	"github.com/pelletier/go-toml/v2"
)

// in Parse(), replacing toml.Unmarshal(rawData, tomlConf):
dec := toml.NewDecoder(bytes.NewReader(rawData))
dec.DisallowUnknownFields()

if err := dec.Decode(tomlConf); err != nil {
	var strict *toml.StrictMissingError
	if errors.As(err, &strict) {
		return nil, fmt.Errorf("cannot parse toml config:\n%s", strict.String())
	}

	return nil, fmt.Errorf("cannot parse toml config: %w", err)
}

What the user then sees on a typo'd key (prxoies instead of proxies) — verified against the pinned go-toml/v2 v2.3.0:

cannot parse toml config:
1| secret = "x"
2| bind-to = "0.0.0.0:3128"
3| prxoies = []
 | ~~~~~~~ missing field

That's the "hard to deduce" pain from the report, gone.

Concrete payoff: this would have turned #524 into a one-line startup error. That reporter put proxies at the top level instead of under [network]; strict mode flags it immediately with the line number, instead of mtg silently starting with no proxy at all.

Verified before proposing:

• example.config.toml parses clean under strict mode — no self-inflicted breakage.
• All well-formed fixtures in internal/config/testdata/ use only known keys — no test fallout (broken.toml is a syntax-error fixture, unaffected).
• Deprecated-but-still-handled keys (domain-fronting-ip, [domain-fronting] ip) stay in the tomlConfig struct, so configs carrying them keep working.

The one trade-off, explicitly: a user upgrading with a config that has a fully removed key (not merely deprecated) now gets a hard startup error instead of silent ignore. That's the intended behavior from the report ("abort the app start (preferable)"), but it's a behavior change worth your sign-off.

Alternative if you'd rather not hard-break: warn-only. I'd advise against it — the logger is built from *config.Config after Parse() returns, so warnings would have to be threaded back through the return signature, and startup-log lines are easy to miss (which is how #514 got hard to deduce in the first place).

Plain TOML syntax errors keep their current terse rendering; surfacing those via .String() too is a small bonus I can fold in or leave out — your call.

Green light for strict-abort, or prefer warn-only / leave as-is?