Bug description
thv run prompts for a required, non-secret environment variable with fmt.Scanln, which reads a single whitespace-delimited token.
When the value contains a space the read fails with expected newline, pkg/runner/env.go:137-143 logs slog.Warn("failed to read input", ...) and continues, and the workload is created without a variable the registry declares required: true.
The unread remainder stays in the stream, so the next prompt reads a fragment of the previous answer.
This is reachable with the catalog this build ships (github.com/stacklok/toolhive-catalog v0.20261005.0, pinned at go.mod:70): it declares 43 required non-secret variable entries, 23 distinct names, and two of them are documented as space-separated by nature, OKTA_SCOPES — "Space-separated OAuth 2.0 scopes granted to the application" — which is exactly the input shape fmt.Scanln cannot read.
CLICKHOUSE_USER, BOX_CLIENT_ID, GRAFANA_URL, ES_URL, DOLT_HOST and KION_SERVER_URL are other required non-secret entries in the same set.
Steps to reproduce
I could not run the CLI end to end here: thv run needs a container runtime and this machine has neither Docker nor Podman.
What I ran is the same function the prompt path calls, with os.Stdin swapped for a pipe, no mocking of the reading itself.
- Check out
main at 2c69e1f1.
- Add the
TestCLIEnvVarValidator_PromptsRequiredEnvVars cases from the PR I am about to open, and run go test -count=1 ./pkg/runner/ -run TestCLIEnvVarValidator_PromptsRequiredEnvVars.
- All five subtests fail. Two of them show the silent path:
WARN failed to read input name=OKTA_SCOPES error="...: expected newline", then Not equal: expected map[string]string{...} actual map[ len=1 ].
A standalone stdlib probe of fmt.Scanln shows both halves on 2c69e1f1:
input "hello world\nsecond-value\n" to two consecutive prompts
first: value="hello" err=expected newline
second: value="orld" err=<nil>
input "\n" -> err=unexpected newline
input with EOF -> err=EOF
so the first required variable is dropped and the second is set to garbage from the leftover, with only a warning for the first.
Expected behavior
Required means required. Either the whole value is captured, or thv run stops with an error naming the variable.
The sibling implementation already does the latter: DetachedEnvVarValidator.Validate returns missing required environment variable: %s (pkg/runner/env.go:57), and pkg/runner/env_test.go:39 pins "required non-secret not provided returns error".
EnvVar.Required is documented in toolhive-core as "If true and not provided via command line or secrets, the user will be prompted for a value."
Actual behavior
The variable is silently missing from the created workload, or silently set to a fragment of the previous answer, while the run continues.
Nothing re-checks required variables after this point, so the misconfigured container really is started.
Environment (if relevant)
- OS/version: macOS 27.2, arm64
- ToolHive version:
main at 2c69e1f1
Additional context
The secret branch of the same function reads a whole line through term.ReadPassword, so only the non-secret branch mangles spaces.
Every other interactive prompt in this repo reads a line with bufio and returns an error: cmd/thv/app/upgrade.go:286, secret.go:469, group.go:249, skill_confirm.go:44, skill_push.go:96. pkg/runner/env.go:169 is the last fmt.Scanln.
748d43c5 ("Fix DetachedEnvVarValidator rejecting optional secret env vars (#5689)") already aligned the two validators on the required/optional question, so this pulls the other half of that file in line.
I am going to implement this and open a PR referencing this issue.
Bug description
thv runprompts for a required, non-secret environment variable withfmt.Scanln, which reads a single whitespace-delimited token.When the value contains a space the read fails with
expected newline,pkg/runner/env.go:137-143logsslog.Warn("failed to read input", ...)and continues, and the workload is created without a variable the registry declaresrequired: true.The unread remainder stays in the stream, so the next prompt reads a fragment of the previous answer.
This is reachable with the catalog this build ships (
github.com/stacklok/toolhive-catalog v0.20261005.0, pinned atgo.mod:70): it declares 43 required non-secret variable entries, 23 distinct names, and two of them are documented as space-separated by nature,OKTA_SCOPES— "Space-separated OAuth 2.0 scopes granted to the application" — which is exactly the input shapefmt.Scanlncannot read.CLICKHOUSE_USER,BOX_CLIENT_ID,GRAFANA_URL,ES_URL,DOLT_HOSTandKION_SERVER_URLare other required non-secret entries in the same set.Steps to reproduce
I could not run the CLI end to end here:
thv runneeds a container runtime and this machine has neither Docker nor Podman.What I ran is the same function the prompt path calls, with
os.Stdinswapped for a pipe, no mocking of the reading itself.mainat2c69e1f1.TestCLIEnvVarValidator_PromptsRequiredEnvVarscases from the PR I am about to open, and rungo test -count=1 ./pkg/runner/ -run TestCLIEnvVarValidator_PromptsRequiredEnvVars.WARN failed to read input name=OKTA_SCOPES error="...: expected newline", thenNot equal: expected map[string]string{...} actual map[ len=1 ].A standalone stdlib probe of
fmt.Scanlnshows both halves on2c69e1f1:so the first required variable is dropped and the second is set to garbage from the leftover, with only a warning for the first.
Expected behavior
Required means required. Either the whole value is captured, or
thv runstops with an error naming the variable.The sibling implementation already does the latter:
DetachedEnvVarValidator.Validatereturnsmissing required environment variable: %s(pkg/runner/env.go:57), andpkg/runner/env_test.go:39pins "required non-secret not provided returns error".EnvVar.Requiredis documented intoolhive-coreas "If true and not provided via command line or secrets, the user will be prompted for a value."Actual behavior
The variable is silently missing from the created workload, or silently set to a fragment of the previous answer, while the run continues.
Nothing re-checks required variables after this point, so the misconfigured container really is started.
Environment (if relevant)
mainat2c69e1f1Additional context
The secret branch of the same function reads a whole line through
term.ReadPassword, so only the non-secret branch mangles spaces.Every other interactive prompt in this repo reads a line with
bufioand returns an error:cmd/thv/app/upgrade.go:286,secret.go:469,group.go:249,skill_confirm.go:44,skill_push.go:96.pkg/runner/env.go:169is the lastfmt.Scanln.748d43c5("Fix DetachedEnvVarValidator rejecting optional secret env vars (#5689)") already aligned the two validators on the required/optional question, so this pulls the other half of that file in line.I am going to implement this and open a PR referencing this issue.