Enforce real timeouts on gpg subprocess calls #62

New Issue

clawbot · 2026-08-09T03:38:22+02:00

clawbot commented

2026-08-09 03:38:22 +02:00

Context

mfer/gpg.go:52-53 runs every gpg invocation as:

exec.CommandContext(context.Background(), "gpg", fullArgs...)

exec.CommandContext is the right call, but context.Background() never
expires, so no deadline is actually enforced. All five gpg call sites
route through runGPG and inherit this: gpgSign, gpgExportPublicKey,
gpgGetKeyFingerprint, gpgExtractPubKeyFingerprint, gpgVerify.

This is a real hang risk, not a theoretical one: gpg blocks indefinitely
waiting on a passphrase prompt, a stalled gpg-agent, or entropy. In a
non-interactive context — CI, a cron job, a server embedding this library —
that is an unbounded hang with no way out.

It is also the most likely cause of the intermittent test-suite timeout
described in the script/test issue: the gpg tests generate real RSA-2048
keys, and a slow keygen has nothing to bound it.

Definition of done

Every gpg subprocess call runs under a context with a real deadline.
The timeout is a named constant, not a magic number, and is documented as
to why that value was chosen.
Callers that already have a context.Context in scope propagate it
instead of manufacturing a fresh background context, so cancellation from
above works.
When the deadline fires, the returned error clearly says the gpg
invocation timed out and which operation it was — not a bare
context deadline exceeded.
Test coverage: a test that a gpg call which exceeds its deadline returns
the timeout error rather than hanging. Use a stub or a deliberately
slow fake binary; do not add a multi-second sleep to the suite.
make check passes. TODO.md updated in the same commit.

Implementation requirements

Do not paper over this by raising the go test timeout. The production
code path is what is unbounded.
Check whether killing the context actually kills gpg. CommandContext
sends os.Interrupt/Kill to the direct child only; if gpg has spawned
or delegated to a gpg-agent, the child may not die. If a process group
kill is needed, do it properly and explain in a comment.
There is an existing //nolint on one of these exec sites referring to a
"non-cancellable signing exec". If signing genuinely cannot be cancelled,
that claim needs to be justified in a comment or withdrawn — it should not
survive as an unexplained suppression.
Preserve current behavior on the success path exactly; this is a
robustness fix, not a redesign. Replacing gpg with pure-Go crypto is a
separate, owner-gated decision and is out of scope here.
Commit title must end with (closes #62).

## Context `mfer/gpg.go:52-53` runs every gpg invocation as: ```go exec.CommandContext(context.Background(), "gpg", fullArgs...) ``` `exec.CommandContext` is the right call, but `context.Background()` never expires, so **no deadline is actually enforced**. All five gpg call sites route through `runGPG` and inherit this: `gpgSign`, `gpgExportPublicKey`, `gpgGetKeyFingerprint`, `gpgExtractPubKeyFingerprint`, `gpgVerify`. This is a real hang risk, not a theoretical one: `gpg` blocks indefinitely waiting on a passphrase prompt, a stalled `gpg-agent`, or entropy. In a non-interactive context — CI, a cron job, a server embedding this library — that is an unbounded hang with no way out. It is also the most likely cause of the intermittent test-suite timeout described in the `script/test` issue: the gpg tests generate real RSA-2048 keys, and a slow keygen has nothing to bound it. ## Definition of done - Every gpg subprocess call runs under a context with a real deadline. - The timeout is a named constant, not a magic number, and is documented as to why that value was chosen. - Callers that already have a `context.Context` in scope propagate it instead of manufacturing a fresh background context, so cancellation from above works. - When the deadline fires, the returned error clearly says the gpg invocation timed out and which operation it was — not a bare `context deadline exceeded`. - Test coverage: a test that a gpg call which exceeds its deadline returns the timeout error rather than hanging. Use a stub or a deliberately slow fake binary; do not add a multi-second sleep to the suite. - `make check` passes. `TODO.md` updated in the same commit. ## Implementation requirements - Do not paper over this by raising the `go test` timeout. The production code path is what is unbounded. - Check whether killing the context actually kills `gpg`. `CommandContext` sends `os.Interrupt`/`Kill` to the direct child only; if `gpg` has spawned or delegated to a `gpg-agent`, the child may not die. If a process group kill is needed, do it properly and explain in a comment. - There is an existing `//nolint` on one of these exec sites referring to a "non-cancellable signing exec". If signing genuinely cannot be cancelled, that claim needs to be justified in a comment or withdrawn — it should not survive as an unexplained suppression. - Preserve current behavior on the success path exactly; this is a robustness fix, not a redesign. Replacing gpg with pure-Go crypto is a separate, owner-gated decision and is out of scope here. - Commit title must end with ` (closes #62)`.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:38:22 +02:00

clawbot referenced this issue

2026-08-09 03:40:00 +02:00

Rewrite script/test to the canonical pattern (30s timeout, -race -cover, conditional verbose rerun) #67

clawbot referenced this issue

2026-08-09 03:44:20 +02:00

DECISION: signature scheme questions blocking 1.0 #82

clawbot referenced this issue

2026-08-09 07:39:41 +02:00

script/cibuild reports a green it did not earn: make check is served from Docker cache #89

clawbot referenced this issue

2026-08-09 07:41:07 +02:00