Logger writes to stdout, so any warning corrupts --json output #82

New Issue

clawbot · 2026-08-09T05:16:09+02:00

clawbot commented

2026-08-09 05:16:09 +02:00

Found while implementing #64. Filing rather than fixing drive-by, since
it is outside that issue's scope and affects every --json command.

Problem

internal/log/log.go:73-78 attaches both handlers to os.Stdout:

if term.IsTerminal(int(os.Stdout.Fd())) {
    logger = slog.New(NewTTYHandler(os.Stdout, opts))
} else {
    logger = slog.New(slog.NewJSONHandler(os.Stdout, opts))
}

Every --json subcommand writes its document to the same stream. Any
log record that is not suppressed therefore lands in the middle of the
document and makes it unparseable. WARN and ERROR are never
suppressed, so this is not hypothetical.

Reproduction

Any config file with permissions looser than 0600 triggers a WARN at
internal/config/config.go:271 during startup:

$ vaultik --config /tmp/demo/config.yml snapshot list --json
{"time":"...","level":"WARN","msg":"Config file has insecure permissions (contains S3 credentials)",...}
[
  {
    "id": "",
    ...
  }
]

vaultik snapshot list --json | jq . fails on that first line. The same
applies to snapshot verify --json, snapshot remove --json, prune --json, and remote info --json.

Expected

Diagnostics go to stderr; stdout carries only the requested output. That
is the usual contract and the one a caller piping to jq relies on.

Notes

internal/cli already passes a Quiet flag through for some
--json paths, which suppresses INFO but not WARN/ERROR, so
quiet mode is not a fix.
snapshot list --json works around this locally as of #64: its
remote-listing warning is written to v.Stderr directly rather than
through log or v.UI. That workaround should be removed once the
logger itself writes to stderr.
Fixing this centrally is a one-line change to the handler
destination, but it will move all --verbose/--debug output to
stderr too, which is a user-visible change worth confirming
deliberately.

Found while implementing #64. Filing rather than fixing drive-by, since it is outside that issue's scope and affects every `--json` command. ## Problem `internal/log/log.go:73-78` attaches both handlers to `os.Stdout`: ```go if term.IsTerminal(int(os.Stdout.Fd())) { logger = slog.New(NewTTYHandler(os.Stdout, opts)) } else { logger = slog.New(slog.NewJSONHandler(os.Stdout, opts)) } ``` Every `--json` subcommand writes its document to the same stream. Any log record that is not suppressed therefore lands in the middle of the document and makes it unparseable. `WARN` and `ERROR` are never suppressed, so this is not hypothetical. ## Reproduction Any config file with permissions looser than 0600 triggers a `WARN` at `internal/config/config.go:271` during startup: ``` $ vaultik --config /tmp/demo/config.yml snapshot list --json {"time":"...","level":"WARN","msg":"Config file has insecure permissions (contains S3 credentials)",...} [ { "id": "", ... } ] ``` `vaultik snapshot list --json | jq .` fails on that first line. The same applies to `snapshot verify --json`, `snapshot remove --json`, `prune --json`, and `remote info --json`. ## Expected Diagnostics go to stderr; stdout carries only the requested output. That is the usual contract and the one a caller piping to `jq` relies on. ## Notes - `internal/cli` already passes a `Quiet` flag through for some `--json` paths, which suppresses `INFO` but not `WARN`/`ERROR`, so quiet mode is not a fix. - `snapshot list --json` works around this locally as of #64: its remote-listing warning is written to `v.Stderr` directly rather than through `log` or `v.UI`. That workaround should be removed once the logger itself writes to stderr. - Fixing this centrally is a one-line change to the handler destination, but it will move all `--verbose`/`--debug` output to stderr too, which is a user-visible change worth confirming deliberately.

Logger writes to stdout, so any warning corrupts --json output #82

Problem

Reproduction

Expected

Notes

Implementation plan

1. Move the logger to stderr

2. Remove the snapshot_list.go workaround

3. Verification

Out of scope

What changed

User-visible change

Verification

One thing this does not finish

2. Remove the `snapshot_list.go` workaround