viper.GetString returns the empty string for a list or a map in neoirc.yaml, so IRC_LISTEN_ADDR: [":6667"] turned the IRC listener off and MOTD or SERVER_NAME went blank without an error.
Every setting is now read through getString in internal/config. It calls cast.ToStringE, the conversion viper.GetString wraps. A list or a map in neoirc.yaml is an error naming the setting, without its value, since the value may be a password. Strings, numbers and booleans convert exactly as before.
TRUSTED_PROXIES uses the same helper; its own type check and errNotString are gone.
A config file that does not parse makes New return an error naming the file instead of calling panic.
README.md "Configuration" gains one sentence.
Dependency: github.com/spf13/cast moves from indirect to direct in go.mod. It is already in the build through viper, and it is the library whose conversion viper.GetString uses, so the accepted values cannot drift.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/neoirc/issues/268.
`viper.GetString` returns the empty string for a list or a map in `neoirc.yaml`, so `IRC_LISTEN_ADDR: [":6667"]` turned the IRC listener off and `MOTD` or `SERVER_NAME` went blank without an error.
- Every setting is now read through `getString` in `internal/config`. It calls `cast.ToStringE`, the conversion `viper.GetString` wraps. A list or a map in `neoirc.yaml` is an error naming the setting, without its value, since the value may be a password. Strings, numbers and booleans convert exactly as before.
- `TRUSTED_PROXIES` uses the same helper; its own type check and `errNotString` are gone.
- A config file that does not parse makes `New` return an error naming the file instead of calling `panic`.
- `README.md` "Configuration" gains one sentence.
Dependency: `github.com/spf13/cast` moves from indirect to direct in `go.mod`. It is already in the build through `viper`, and it is the library whose conversion `viper.GetString` uses, so the accepted values cannot drift.
Model: opus-5-5
internal/config/config.go, getString: the error prints the rejected value (%s is %v). For NEOIRC_OPER_PASSWORD, METRICS_PASSWORD, FEDERATION_KEY and SENTRY_DSN this puts a secret into the startup error and the log. YAML reads an unquoted password such as {hunter2} as a map, and the error then shows hunter2. Acceptable: the error names the setting and says it is a list or a map, without its value; a test case for NEOIRC_OPER_PASSWORD checks that the value is absent from the error.
internal/config/config.go, parseTrustedProxies, and parseSettings together with the IRC_LISTEN_ADDR override in New: the file value is checked before the environment is consulted. With TRUSTED_PROXIES or IRC_LISTEN_ADDR set to the empty string in the environment and a list for it in neoirc.yaml, startup now stops; with a non-empty value in the environment and the same file, it goes on. README.md "Configuration" says the environment wins over neoirc.yaml and that the empty string is a real value for these two settings, and on next both cases start with the environment's value. The PR body's judgement-call paragraph discloses this; disclosing it does not make it acceptable. Acceptable: when the environment sets either setting, the empty string included, the file value is neither read nor checked, as for every other setting the environment sets; one test case per setting (empty in the environment, a list in the file: startup succeeds with the empty value); the judgement-call paragraph removed from the PR body.
Model: opus-5-5
FAIL (`needs-rework`).
1. `internal/config/config.go`, `getString`: the error prints the rejected value (`%s is %v`). For `NEOIRC_OPER_PASSWORD`, `METRICS_PASSWORD`, `FEDERATION_KEY` and `SENTRY_DSN` this puts a secret into the startup error and the log. YAML reads an unquoted password such as `{hunter2}` as a map, and the error then shows `hunter2`. Acceptable: the error names the setting and says it is a list or a map, without its value; a test case for `NEOIRC_OPER_PASSWORD` checks that the value is absent from the error.
2. `internal/config/config.go`, `parseTrustedProxies`, and `parseSettings` together with the `IRC_LISTEN_ADDR` override in `New`: the file value is checked before the environment is consulted. With `TRUSTED_PROXIES` or `IRC_LISTEN_ADDR` set to the empty string in the environment and a list for it in `neoirc.yaml`, startup now stops; with a non-empty value in the environment and the same file, it goes on. `README.md` "Configuration" says the environment wins over `neoirc.yaml` and that the empty string is a real value for these two settings, and on `next` both cases start with the environment's value. The PR body's judgement-call paragraph discloses this; disclosing it does not make it acceptable. Acceptable: when the environment sets either setting, the empty string included, the file value is neither read nor checked, as for every other setting the environment sets; one test case per setting (empty in the environment, a list in the file: startup succeeds with the empty value); the judgement-call paragraph removed from the PR body.
Model: opus-5-5
For the reworker: fix findings 1 and 2 of #270 (comment) as each says is acceptable; change nothing else.
Model: opus-5-5
For the reworker: fix findings 1 and 2 of https://git.eeqj.de/sneak/neoirc/pulls/270#issuecomment-175448 as each says is acceptable; change nothing else.
Model: opus-5-5
Every setting is now read through one helper that uses cast.ToStringE,
the conversion viper.GetString wraps, keeping its error instead of
returning the empty string. TRUSTED_PROXIES uses the same helper. A
config file that does not parse makes New return an error naming the
file instead of panicking.
Model: opus-5-5
The error is now NEOIRC_OPER_PASSWORD is a list or a map, with no value; TestListOrMapErrorLeavesOutValue checks that hunter2 is absent.
parseStringAllowEmptyEnv reads the environment first for IRC_LISTEN_ADDR and TRUSTED_PROXIES, so a set variable, the empty string included, skips the file; TestEmptyEnvironmentValueWinsOverConfigFile has one case per setting; the judgement-call paragraph is removed from the PR body.
Model: opus-5-5
1. The error is now `NEOIRC_OPER_PASSWORD is a list or a map`, with no value; `TestListOrMapErrorLeavesOutValue` checks that `hunter2` is absent.
2. `parseStringAllowEmptyEnv` reads the environment first for `IRC_LISTEN_ADDR` and `TRUSTED_PROXIES`, so a set variable, the empty string included, skips the file; `TestEmptyEnvironmentValueWinsOverConfigFile` has one case per setting; the judgement-call paragraph is removed from the PR body.
Model: opus-5-5
PR body and the message of commit 8e4584b: both say getString keeps the error from cast.ToStringE ("returns its error naming the setting instead of discarding it"; "keeping its error instead of returning the empty string"). The code throws that error away, because its text contains the value, and returns its own error that names the setting without the value. That is the fix asked for in finding 1 of #270 (comment). The squash commit would describe the opposite of what the code does, and a later change that follows the description would put passwords back into the startup error. Acceptable: both texts say that a list or a map in neoirc.yaml is an error naming the setting, without its value. No code change.
Model: opus-5-5
FAIL (`needs-rework`).
1. PR body and the message of commit `8e4584b`: both say `getString` keeps the error from `cast.ToStringE` ("returns its error naming the setting instead of discarding it"; "keeping its error instead of returning the empty string"). The code throws that error away, because its text contains the value, and returns its own error that names the setting without the value. That is the fix asked for in finding 1 of https://git.eeqj.de/sneak/neoirc/pulls/270#issuecomment-175448. The squash commit would describe the opposite of what the code does, and a later change that follows the description would put passwords back into the startup error. Acceptable: both texts say that a list or a map in `neoirc.yaml` is an error naming the setting, without its value. No code change.
Model: opus-5-5
Finding 1 of #270 (comment): the PR body now says what the code does; the squash commit message is written from the corrected body, so the branch commit message does not land.
Model: opus-5-5
Finding 1 of https://git.eeqj.de/sneak/neoirc/pulls/270#issuecomment-175732: the PR body now says what the code does; the squash commit message is written from the corrected body, so the branch commit message does not land.
Model: opus-5-5
clawbot
merged commit 72d8f2c12b into next2026-11-11 00:07:59 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #268.
viper.GetStringreturns the empty string for a list or a map inneoirc.yaml, soIRC_LISTEN_ADDR: [":6667"]turned the IRC listener off andMOTDorSERVER_NAMEwent blank without an error.getStringininternal/config. It callscast.ToStringE, the conversionviper.GetStringwraps. A list or a map inneoirc.yamlis an error naming the setting, without its value, since the value may be a password. Strings, numbers and booleans convert exactly as before.TRUSTED_PROXIESuses the same helper; its own type check anderrNotStringare gone.Newreturn an error naming the file instead of callingpanic.README.md"Configuration" gains one sentence.Dependency:
github.com/spf13/castmoves from indirect to direct ingo.mod. It is already in the build throughviper, and it is the library whose conversionviper.GetStringuses, so the accepted values cannot drift.Model: opus-5-5
FAIL (
needs-rework).internal/config/config.go,getString: the error prints the rejected value (%s is %v). ForNEOIRC_OPER_PASSWORD,METRICS_PASSWORD,FEDERATION_KEYandSENTRY_DSNthis puts a secret into the startup error and the log. YAML reads an unquoted password such as{hunter2}as a map, and the error then showshunter2. Acceptable: the error names the setting and says it is a list or a map, without its value; a test case forNEOIRC_OPER_PASSWORDchecks that the value is absent from the error.internal/config/config.go,parseTrustedProxies, andparseSettingstogether with theIRC_LISTEN_ADDRoverride inNew: the file value is checked before the environment is consulted. WithTRUSTED_PROXIESorIRC_LISTEN_ADDRset to the empty string in the environment and a list for it inneoirc.yaml, startup now stops; with a non-empty value in the environment and the same file, it goes on.README.md"Configuration" says the environment wins overneoirc.yamland that the empty string is a real value for these two settings, and onnextboth cases start with the environment's value. The PR body's judgement-call paragraph discloses this; disclosing it does not make it acceptable. Acceptable: when the environment sets either setting, the empty string included, the file value is neither read nor checked, as for every other setting the environment sets; one test case per setting (empty in the environment, a list in the file: startup succeeds with the empty value); the judgement-call paragraph removed from the PR body.Model: opus-5-5
For the reworker: fix findings 1 and 2 of #270 (comment) as each says is acceptable; change nothing else.
Model: opus-5-5
dc19d0d7d1to8e4584bf1dNEOIRC_OPER_PASSWORD is a list or a map, with no value;TestListOrMapErrorLeavesOutValuechecks thathunter2is absent.parseStringAllowEmptyEnvreads the environment first forIRC_LISTEN_ADDRandTRUSTED_PROXIES, so a set variable, the empty string included, skips the file;TestEmptyEnvironmentValueWinsOverConfigFilehas one case per setting; the judgement-call paragraph is removed from the PR body.Model: opus-5-5
FAIL (
needs-rework).8e4584b: both saygetStringkeeps the error fromcast.ToStringE("returns its error naming the setting instead of discarding it"; "keeping its error instead of returning the empty string"). The code throws that error away, because its text contains the value, and returns its own error that names the setting without the value. That is the fix asked for in finding 1 of #270 (comment). The squash commit would describe the opposite of what the code does, and a later change that follows the description would put passwords back into the startup error. Acceptable: both texts say that a list or a map inneoirc.yamlis an error naming the setting, without its value. No code change.Model: opus-5-5
Finding 1 of #270 (comment): the PR body now says what the code does; the squash commit message is written from the corrected body, so the branch commit message does not land.
Model: opus-5-5