The gateway passed --print-timeout 120 to a CLI that expected 120s. Its argument test also expected 120, so the test passed while the command failed during flag parsing.
The formatter and the test agreed. Neither established what the CLI accepted.
The missing unit
The Go service wraps a CLI with a fixed argument list. Callers can supply a prompt and a bounded timeout; the server chooses the executable, agent, output format, and other options.
It uses exec.Command without an intervening shell. That keeps shell interpretation out of this path, though it doesn’t remove the need to validate inputs or understand how the CLI handles them.
The timeout formatter converted a duration to whole seconds, rounding up and enforcing a minimum of one second. It then returned the number as a string.
The fix retained that behavior and added the unit:
return strconv.FormatInt(seconds, 10) + "s"The command’s duration parser rejected 120 with a missing-unit error. 120s expressed the intended value.
Go’s time.ParseDuration accepts unit-bearing strings such as 300ms and 2m. The special value 0 is also accepted without a unit, so “bare integers never work” would be too broad. The positive values this formatter produced needed the suffix.
The assertion repeated the mistake
The test compared the generated arguments with an expected list containing:
--print-timeout 5That was useful coverage of argument assembly. The service deliberately owns most of the command line, and a test can catch an accidental change to those fixed options.
But this expected value came from the same assumption as the formatter: the timeout flag takes a number of seconds.
There was no independent check against the parser on the other side. The test could detect a change away from the expected string while preserving the wrong string indefinitely.
A default displayed as 5m0s in the CLI’s help was a useful clue. It wasn’t a substitute for checking the flag’s contract or exercising the actual parser.
The regression cases covered exact seconds, fractional seconds rounding up, and the minimum: 120s, 2s for a 1500ms input, and 1s for zero. Those cases check the formatter’s policy. A small integration check with the supported CLI version adds evidence that the resulting arguments are accepted.
Diagnosis still needs to respect redaction
The gateway returned a generic subprocess failure rather than exposing captured stderr. That kept raw CLI diagnostics out of public errors and structured logs, where they could reveal prompts, tokens, or authentication state.
It also meant the ordinary error message didn’t contain the missing-unit explanation.
I wouldn’t resolve that by logging raw stderr everywhere internally. Internal logs can leak secrets too. A controlled reproduction with synthetic input, or a narrowly defined safe diagnostic, is a better way to investigate without weakening the default policy.
An early exit with empty stdout can suggest a startup or parsing problem, but it doesn’t identify the cause by itself. In this case, the parser’s error supplied the evidence.
The argument test was worth keeping. It just needed a companion check at the boundary with the CLI. An exact match to the expected command line doesn’t help when the expected command line is wrong.
Sources
- Go time.ParseDuration — duration-string syntax.
- Go flag.Duration — duration-valued command-line flags.
- Go os/exec — subprocess execution without implicit shell interpretation.
I’d appreciate a follow. You can subscribe with your email below. The emails go out once a week, or you can find me on Mastodon at @[email protected].