errors: reject invalid TSA responses and preserve error chains #8

New issue

Closed

opened 2026-07-21 17:20:18 +02:00 by heiko · 0 comments

heiko commented

2026-07-21 17:20:18 +02:00

Owner

Problem

Error handling is inconsistent in security- and output-sensitive paths:

sign.go prints Response.Validate failures as ?? ... to stdout and continues. This can both accept invalid responses and corrupt an RFC 5322 message or DER stream.
tsr.Format failures are sent through verbosef, which may be disabled, and processing continues.
Some wrapped errors use %v instead of %w (sign.go client creation, internal/tsr.Base64), preventing errors.Is/errors.As inspection.
query.go returns dependency errors without operation context.

Checklist: #49 (wrap errors intentionally), #52 (do not log and return/continue ambiguously), #53 (do not suppress required errors).

Proposed fix

Treat response/request validation failures as rejection: do not emit that timestamp.
Never write diagnostics to the primary stdout stream.
Decide which formatting is required versus optional; required failures return an error, optional diagnostics are clearly labeled on stderr.
Wrap actionable lower-level errors with %w and operation/endpoint context.
Aggregate per-TSA failures when no acceptable response is obtained.

Acceptance criteria

Invalid TSA responses produce no primary output and a nonzero exit.
Binary output remains byte-pure under all failures.
Callers can use errors.Is/errors.As through wrapped errors.
Tests inject validation, formatting, writer, and network failures and assert both streams.

Reviewed against 948b7a9 on master (Go 1.26.2).

## Problem Error handling is inconsistent in security- and output-sensitive paths: - `sign.go` prints `Response.Validate` failures as `?? ...` to stdout and continues. This can both accept invalid responses and corrupt an RFC 5322 message or DER stream. - `tsr.Format` failures are sent through `verbosef`, which may be disabled, and processing continues. - Some wrapped errors use `%v` instead of `%w` (`sign.go` client creation, `internal/tsr.Base64`), preventing `errors.Is`/`errors.As` inspection. - `query.go` returns dependency errors without operation context. Checklist: #49 (wrap errors intentionally), #52 (do not log and return/continue ambiguously), #53 (do not suppress required errors). ## Proposed fix - Treat response/request validation failures as rejection: do not emit that timestamp. - Never write diagnostics to the primary stdout stream. - Decide which formatting is required versus optional; required failures return an error, optional diagnostics are clearly labeled on stderr. - Wrap actionable lower-level errors with `%w` and operation/endpoint context. - Aggregate per-TSA failures when no acceptable response is obtained. ## Acceptance criteria - Invalid TSA responses produce no primary output and a nonzero exit. - Binary output remains byte-pure under all failures. - Callers can use `errors.Is`/`errors.As` through wrapped errors. - Tests inject validation, formatting, writer, and network failures and assert both streams. Reviewed against `948b7a9` on `master` (Go 1.26.2).

heiko referenced this issue from a commit

2026-07-22 11:18:25 +02:00

docs+test: describe implemented verification; add verify-failure tests

heiko referenced this issue from a commit

2026-07-22 11:18:25 +02:00

fix(errors): add query context and TSA validation-failure tests (closes #8)

heiko closed this issue

2026-07-22 11:18:25 +02:00