Enforce real timeouts on gpg subprocess calls (closes #62)
check / check (push) Successful in 2m2s
check / check (push) Successful in 2m2s
Every gpg run now has a one-minute deadline (gpgTimeout) on top of its caller's context and is killed when either ends. A timeout is reported as "gpg timed out" under the failing operation instead of "signal: killed". Only gpg itself is killed; WaitDelay (one second) stops the run from waiting on a process gpg left behind that still holds its output, such as a wrapper script that does not exec the real gpg. Builder.Build and Checker.ExtractEmbeddedSigningKeyFP take a context, so ToManifest's context now reaches signing and the contextcheck suppression calling signing non-cancellable is gone. Manifest loading takes no context, so its signature check is bounded by the timeout alone. Model: opus-5-5
This commit was merged in pull request #119.
This commit is contained in:
+48
-15
@@ -10,9 +10,21 @@ import (
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
const (
|
||||
// gpgTimeout bounds every gpg run, which can otherwise wait forever on
|
||||
// a passphrase prompt or a stalled gpg-agent. A minute leaves a person
|
||||
// time to type a passphrase or touch a smartcard.
|
||||
gpgTimeout = time.Minute
|
||||
|
||||
// gpgWaitDelay is how long a gpg run keeps waiting for gpg's stdout
|
||||
// and stderr to close once gpg has been killed or has exited. Reading
|
||||
// what gpg itself wrote takes far less; only a process gpg left behind
|
||||
// holds them open longer.
|
||||
gpgWaitDelay = time.Second
|
||||
|
||||
// privateDirPerms is the permission mode for temporary GPG home
|
||||
// directories.
|
||||
privateDirPerms os.FileMode = 0o700
|
||||
@@ -66,8 +78,20 @@ func gpgArgs(opts []string, positional ...string) []string {
|
||||
}
|
||||
|
||||
// runGPG runs the gpg binary in batch mode with the given arguments and
|
||||
// optional stdin, returning captured stdout and stderr.
|
||||
func runGPG(stdin io.Reader, args ...string) (*bytes.Buffer, *bytes.Buffer, error) {
|
||||
// optional stdin, returning captured stdout and stderr. gpg is killed when
|
||||
// ctx ends or gpgTimeout passes, whichever comes first.
|
||||
func runGPG(
|
||||
ctx context.Context, stdin io.Reader, args ...string,
|
||||
) (*bytes.Buffer, *bytes.Buffer, error) {
|
||||
// exec.CommandContext kills only gpg itself. A gpg-agent that gpg
|
||||
// starts runs detached and holds none of gpg's output, but another
|
||||
// process gpg leaves behind (a wrapper script that runs the real gpg
|
||||
// without exec, for example) can keep gpg's stdout or stderr open, and
|
||||
// Run would wait for it to exit. WaitDelay stops that wait
|
||||
// gpgWaitDelay after the kill; that process is left running.
|
||||
ctx, cancel := context.WithTimeout(ctx, gpgTimeout)
|
||||
defer cancel()
|
||||
|
||||
fullArgs := append([]string{"--batch", "--no-tty"}, args...)
|
||||
|
||||
// G204: the executable name is a compile-time constant. The arguments
|
||||
@@ -76,7 +100,8 @@ func runGPG(stdin io.Reader, args ...string) (*bytes.Buffer, *bytes.Buffer, erro
|
||||
// option or after the "--" end-of-options marker inserted by gpgArgs,
|
||||
// and therefore cannot be reinterpreted by gpg as an option.
|
||||
cmd := exec.CommandContext( //nolint:gosec // G204: see comment above
|
||||
context.Background(), "gpg", fullArgs...)
|
||||
ctx, "gpg", fullArgs...)
|
||||
cmd.WaitDelay = gpgWaitDelay
|
||||
cmd.Stdin = stdin
|
||||
|
||||
var stdout, stderr bytes.Buffer
|
||||
@@ -85,6 +110,14 @@ func runGPG(stdin io.Reader, args ...string) (*bytes.Buffer, *bytes.Buffer, erro
|
||||
cmd.Stderr = &stderr
|
||||
|
||||
err := cmd.Run()
|
||||
if err != nil && ctx.Err() != nil {
|
||||
// gpg was killed because ctx ended, which Run reports only as
|
||||
// "signal: killed"; return the reason instead.
|
||||
err = ctx.Err()
|
||||
if errors.Is(err, context.DeadlineExceeded) {
|
||||
err = fmt.Errorf("gpg timed out: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
return &stdout, &stderr, err
|
||||
}
|
||||
@@ -105,8 +138,8 @@ func parseFingerprint(colonOutput string) (string, bool) {
|
||||
|
||||
// gpgSign creates a detached signature of the data using the specified key.
|
||||
// Returns the armored detached signature.
|
||||
func gpgSign(data []byte, keyID GPGKeyID) ([]byte, error) {
|
||||
stdout, stderr, err := runGPG(bytes.NewReader(data),
|
||||
func gpgSign(ctx context.Context, data []byte, keyID GPGKeyID) ([]byte, error) {
|
||||
stdout, stderr, err := runGPG(ctx, bytes.NewReader(data),
|
||||
"--detach-sign",
|
||||
gpgOptArmor,
|
||||
"--local-user", string(keyID),
|
||||
@@ -120,8 +153,8 @@ func gpgSign(data []byte, keyID GPGKeyID) ([]byte, error) {
|
||||
|
||||
// gpgExportPublicKey exports the public key for the specified key ID.
|
||||
// Returns the armored public key.
|
||||
func gpgExportPublicKey(keyID GPGKeyID) ([]byte, error) {
|
||||
stdout, stderr, err := runGPG(nil,
|
||||
func gpgExportPublicKey(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
|
||||
stdout, stderr, err := runGPG(ctx, nil,
|
||||
gpgArgs([]string{"--export", gpgOptArmor}, string(keyID))...,
|
||||
)
|
||||
if err != nil {
|
||||
@@ -136,8 +169,8 @@ func gpgExportPublicKey(keyID GPGKeyID) ([]byte, error) {
|
||||
}
|
||||
|
||||
// gpgGetKeyFingerprint gets the full fingerprint for a key ID.
|
||||
func gpgGetKeyFingerprint(keyID GPGKeyID) ([]byte, error) {
|
||||
stdout, stderr, err := runGPG(nil,
|
||||
func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
|
||||
stdout, stderr, err := runGPG(ctx, nil,
|
||||
gpgArgs([]string{"--with-colons", "--fingerprint"}, string(keyID))...,
|
||||
)
|
||||
if err != nil {
|
||||
@@ -157,7 +190,7 @@ func gpgGetKeyFingerprint(keyID GPGKeyID) ([]byte, error) {
|
||||
// gpgExtractPubKeyFingerprint imports a public key into a temporary keyring
|
||||
// and extracts its fingerprint. This verifies the key is valid and returns
|
||||
// the actual fingerprint from the key material.
|
||||
func gpgExtractPubKeyFingerprint(pubKey []byte) (string, error) {
|
||||
func gpgExtractPubKeyFingerprint(ctx context.Context, pubKey []byte) (string, error) {
|
||||
// Create temporary directory for GPG operations
|
||||
tmpDir, err := os.MkdirTemp("", "mfer-gpg-fingerprint-*")
|
||||
if err != nil {
|
||||
@@ -181,7 +214,7 @@ func gpgExtractPubKeyFingerprint(pubKey []byte) (string, error) {
|
||||
}
|
||||
|
||||
// Import the public key into the temporary keyring
|
||||
_, importStderr, err := runGPG(nil,
|
||||
_, importStderr, err := runGPG(ctx, nil,
|
||||
gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)...,
|
||||
)
|
||||
if err != nil {
|
||||
@@ -191,7 +224,7 @@ func gpgExtractPubKeyFingerprint(pubKey []byte) (string, error) {
|
||||
}
|
||||
|
||||
// List keys to get fingerprint
|
||||
listStdout, listStderr, err := runGPG(nil,
|
||||
listStdout, listStderr, err := runGPG(ctx, nil,
|
||||
"--homedir", tmpDir,
|
||||
"--with-colons",
|
||||
"--fingerprint",
|
||||
@@ -212,7 +245,7 @@ func gpgExtractPubKeyFingerprint(pubKey []byte) (string, error) {
|
||||
|
||||
// gpgVerify verifies a detached signature against data using the provided public key.
|
||||
// It creates a temporary keyring to import the public key for verification.
|
||||
func gpgVerify(data, signature, pubKey []byte) error {
|
||||
func gpgVerify(ctx context.Context, data, signature, pubKey []byte) error {
|
||||
// Create temporary directory for GPG operations
|
||||
tmpDir, err := os.MkdirTemp("", "mfer-gpg-verify-*")
|
||||
if err != nil {
|
||||
@@ -252,7 +285,7 @@ func gpgVerify(data, signature, pubKey []byte) error {
|
||||
}
|
||||
|
||||
// Import the public key into the temporary keyring
|
||||
_, importStderr, err := runGPG(nil,
|
||||
_, importStderr, err := runGPG(ctx, nil,
|
||||
gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)...,
|
||||
)
|
||||
if err != nil {
|
||||
@@ -262,7 +295,7 @@ func gpgVerify(data, signature, pubKey []byte) error {
|
||||
}
|
||||
|
||||
// Verify the signature
|
||||
_, verifyStderr, err := runGPG(nil,
|
||||
_, verifyStderr, err := runGPG(ctx, nil,
|
||||
gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptVerify},
|
||||
sigFile, dataFile)...,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user