diff --git a/CHANGELOG.md b/CHANGELOG.md index 47384aa..2ce2eb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ Requests: pin Go 1.27 ML-KEM hybrid CurvePreferences (including P-521 fallback) and print the negotiated key exchange. +### Fix + + Devenv: prefer httpbin on 127.0.0.1:8081 and proxy nginx upstreams through the allocated httpbin port so `devenv test` keeps working when the preferred port is already taken; fail fast in request integration tests with `set -e`, enable `pipefail` on success-case request leaf pipelines, and assert request exit status separately from expected error text. + ### Tests Certinfo and requests: share CA/leaf certificate generation and custom TLS httptest servers via internal/tlstest. diff --git a/devenv.nix b/devenv.nix index bc5c277..66e9091 100644 --- a/devenv.nix +++ b/devenv.nix @@ -100,7 +100,7 @@ in listen [::]:9443 ssl; http2 on; location / { - proxy_pass http://localhost:8080; + proxy_pass http://127.0.0.1:${toString config.processes.httpbin.ports.main.value}; proxy_set_header Host $host; proxy_set_header X-Forwarded-For $remote_addr; } @@ -116,7 +116,7 @@ in listen [::]:9444 ssl proxy_protocol; http2 on; location / { - proxy_pass http://localhost:8080; + proxy_pass http://127.0.0.1:${toString config.processes.httpbin.ports.main.value}; proxy_pass_request_headers on; proxy_set_header Host $host; proxy_set_header X-Proxy-Protocol enabled; @@ -135,7 +135,7 @@ in listen [::]:9445 ssl; http2 on; location / { - proxy_pass http://localhost:8080; + proxy_pass http://127.0.0.1:${toString config.processes.httpbin.ports.main.value}; proxy_set_header Host $host; proxy_set_header X-Forwarded-For $remote_addr; } @@ -150,7 +150,7 @@ in listen [::]:9446 ssl; http2 on; location / { - proxy_pass http://localhost:8080; + proxy_pass http://127.0.0.1:${toString config.processes.httpbin.ports.main.value}; proxy_set_header Host $host; proxy_set_header X-Forwarded-For $remote_addr; } @@ -165,7 +165,7 @@ in listen [::]:9447 ssl; http2 on; location / { - proxy_pass http://localhost:8080; + proxy_pass http://127.0.0.1:${toString config.processes.httpbin.ports.main.value}; proxy_set_header Host $host; proxy_set_header X-Forwarded-For $remote_addr; } @@ -175,6 +175,7 @@ in services.httpbin = { enable = true; + bind = [ "127.0.0.1:8081" ]; }; tasks."web:refreshCertsBeforeNginxStart" = { @@ -325,6 +326,7 @@ in scripts.test-requests-show-sample-config.exec = '' gum format "## test request show sample config" + set -o pipefail ./dist/https-wrench requests --show-sample-config| grep 'requests:' ''; @@ -345,18 +347,29 @@ in scripts.test-requests-timeout.exec = '' gum format "## test request timeout" - time ./dist/https-wrench requests --config ./${config.env.EXAMPLES}/https-wrench-request-timeout.yaml | grep "Client.Timeout exceeded while awaiting headers" + set +e + out=$(./dist/https-wrench requests --config ./${config.env.EXAMPLES}/https-wrench-request-timeout.yaml 2>&1) + status=$? + set -e + printf '%s\n' "$out" | grep "Client.Timeout exceeded while awaiting headers" + # requests prints per-request errors but exits 0 + test "$status" -eq 0 ''; scripts.test-requests-unknown-ca.exec = '' gum format "## test request with unknown CA" - - set +o pipefail - ./dist/https-wrench requests --config ./${config.env.EXAMPLES}/tests-configs/unknown-ca.yaml | grep 'failed to verify certificate: x509: certificate signed by unknown authority' + set +e + out=$(./dist/https-wrench requests --config ./${config.env.EXAMPLES}/tests-configs/unknown-ca.yaml 2>&1) + status=$? + set -e + printf '%s\n' "$out" | grep 'failed to verify certificate: x509: certificate signed by unknown authority' + # requests prints per-request errors but exits 0 + test "$status" -eq 0 ''; scripts.test-requests-insecure.exec = '' gum format "## test request insecure skip verify" + set -o pipefail ./dist/https-wrench requests --config ./${config.env.EXAMPLES}/tests-configs/insecure.yaml | grep 'StatusCode: 200' ''; @@ -367,31 +380,47 @@ in scripts.test-requests-body-regexp-match.exec = '' gum format "## test request body regexp match" + set -o pipefail ./dist/https-wrench requests --config ./${config.env.EXAMPLES}/tests-configs/body-regexp-match.yaml --ca-bundle $CAROOT/rootCA.pem | grep 'BodyRegexpMatch: true' ''; scripts.test-requests-ca-bundle-file-success.exec = '' gum format "## test request with CA bundle file" + set -o pipefail ./dist/https-wrench requests --config ./${config.env.EXAMPLES}/tests-configs/ca-bundle-200.yaml --ca-bundle $CAROOT/rootCA.pem | grep "StatusCode: 200" ''; scripts.test-requests-valid-cert-wrong-ca-bundle.exec = '' gum format "## test request with valid cert and wrong CA bundle file" - ./dist/https-wrench requests --config ./${config.env.EXAMPLES}/tests-configs/repo-os76.yaml --ca-bundle $CAROOT/rootCA.pem 2>&1 | grep 'certificate signed by unknown authority' + set +e + out=$(./dist/https-wrench requests --config ./${config.env.EXAMPLES}/tests-configs/repo-os76.yaml --ca-bundle $CAROOT/rootCA.pem 2>&1) + status=$? + set -e + printf '%s\n' "$out" | grep 'certificate signed by unknown authority' + # requests prints per-request errors but exits 0 + test "$status" -eq 0 ''; scripts.test-requests-ca-bundle-file-wrong-servername.exec = '' gum format "## test request with CA bundle file and wrong host name / servername" - ./dist/https-wrench requests --config ./${config.env.EXAMPLES}/tests-configs/ca-bundle-wrong-servername.yaml --ca-bundle $CAROOT/rootCA.pem | grep 'tls: failed to verify certificate: x509' + set +e + out=$(./dist/https-wrench requests --config ./${config.env.EXAMPLES}/tests-configs/ca-bundle-wrong-servername.yaml --ca-bundle $CAROOT/rootCA.pem 2>&1) + status=$? + set -e + printf '%s\n' "$out" | grep 'tls: failed to verify certificate: x509' + # requests prints per-request errors but exits 0 + test "$status" -eq 0 ''; scripts.test-requests-proxy-protocol-ipv4.exec = '' gum format "## test request proxy protocol IPv4" + set -o pipefail ./dist/https-wrench requests --config ./${config.env.EXAMPLES}/tests-configs/proxy-protocol-ipv4.yaml --ca-bundle $CAROOT/rootCA.pem | grep '192.0.2.1' ''; scripts.test-requests-proxy-protocol-ipv6.exec = '' gum format "## test request proxy protocol IPv6" + set -o pipefail ./dist/https-wrench requests --config ./${config.env.EXAMPLES}/tests-configs/proxy-protocol-ipv6.yaml --ca-bundle $CAROOT/rootCA.pem | grep '2001:db8::1' ''; @@ -405,6 +434,7 @@ in echo "caBundle: |" >> $CA_BUNDLE_YAML_TEST_FILE while IFS= read -r line; do echo " $line" >> $CA_BUNDLE_YAML_TEST_FILE ; done < $CAROOT/rootCA.pem + set -o pipefail ./dist/https-wrench requests --config $CA_BUNDLE_YAML_TEST_FILE | grep 'StatusCode: 200' ''; @@ -623,6 +653,7 @@ in ''; scripts.run-requests-tests.exec = '' + set -e gum format "## Requests tests" # test-requests-sample-config diff --git a/internal/certinfo/certinfo_handlers.go b/internal/certinfo/certinfo_handlers.go index b1c50c5..163c87d 100644 --- a/internal/certinfo/certinfo_handlers.go +++ b/internal/certinfo/certinfo_handlers.go @@ -174,11 +174,7 @@ func (c *Config) printCACerts(w io.Writer, ks, sl, sv lipgloss.Style) error { inputReader, ) if err != nil { - return fmt.Errorf( - "unable for read Root certificates from %s: %w", - c.CACertsFilePath, - err, - ) + return fmt.Errorf("unable to read Root certificates: %w", err) } CertsToTables(w, rootCerts) diff --git a/internal/certinfo/certinfo_handlers_test.go b/internal/certinfo/certinfo_handlers_test.go index e145a4d..2706f8b 100644 --- a/internal/certinfo/certinfo_handlers_test.go +++ b/internal/certinfo/certinfo_handlers_test.go @@ -535,7 +535,7 @@ func TestCertinfo_PrintData(t *testing.T) { errPrint := cc.PrintData(context.Background(), &buffer) require.Error(t, errPrint) - require.ErrorContains(t, errPrint, "unable for read Root certificates") + require.ErrorContains(t, errPrint, "unable to read Root certificates") }) } diff --git a/internal/certinfo/certinfo_test.go b/internal/certinfo/certinfo_test.go index 8177619..ef5346e 100644 --- a/internal/certinfo/certinfo_test.go +++ b/internal/certinfo/certinfo_test.go @@ -53,7 +53,7 @@ var certinfoConfigFileReadErrorTests = []struct { expectMsg: map[string]string{ "caPool": "failed to read CA bundle file: unable to read file testdata/unreadable-file.txt", "certs": "error reading certificate file: unable to read file testdata/unreadable-file.txt", - "key": "unable to read file testdata/unreadable-file.txt", + "key": "error reading private key file: unable to read file testdata/unreadable-file.txt", }, }, { @@ -66,7 +66,7 @@ var certinfoConfigFileReadErrorTests = []struct { expectMsg: map[string]string{ "caPool": "failed to read CA bundle file: open testdata/not-exist: no such file or directory", "certs": "error reading certificate file: open testdata/not-exist: no such file or directory", - "key": "open testdata/not-exist: no such file or directory", + "key": "error reading private key file: open testdata/not-exist: no such file or directory", }, }, { diff --git a/internal/certinfo/common_handlers.go b/internal/certinfo/common_handlers.go index 643a5f1..6119daa 100644 --- a/internal/certinfo/common_handlers.go +++ b/internal/certinfo/common_handlers.go @@ -4,7 +4,6 @@ import ( "crypto" "crypto/x509" "encoding/pem" - "errors" "fmt" "io" "os" @@ -42,7 +41,7 @@ func certMatchPrivateKey(cert *x509.Certificate, key crypto.PrivateKey) (bool, e pub, ok := cert.PublicKey.(interface{ Equal(crypto.PublicKey) bool }) if !ok { - return false, errors.New("unsupported public key type in certificate") + return false, ErrUnsupportedPublicKey } signer, ok := key.(crypto.Signer) @@ -56,11 +55,11 @@ func certMatchPrivateKey(cert *x509.Certificate, key crypto.PrivateKey) (bool, e // GetRootCertsFromFile reads a PEM bundle from a file and returns an x509 CertPool. func GetRootCertsFromFile(caBundlePath string, fileReader Reader) (*x509.CertPool, error) { if caBundlePath == emptyString { - return nil, errors.New("empty string provided as caBundlePath") + return nil, &EmptyArgError{Name: "caBundlePath"} } if fileReader == nil { - return nil, errors.New("nil Reader provided") + return nil, ErrNilReader } certsFromFile, err := fileReader.ReadFile(caBundlePath) @@ -70,7 +69,7 @@ func GetRootCertsFromFile(caBundlePath string, fileReader Reader) (*x509.CertPoo rootCAPool := x509.NewCertPool() if ok := rootCAPool.AppendCertsFromPEM(certsFromFile); !ok { - return nil, errors.New("unable to create CertPool from file") + return nil, ErrCertPoolFromFile } return rootCAPool, nil @@ -79,12 +78,12 @@ func GetRootCertsFromFile(caBundlePath string, fileReader Reader) (*x509.CertPoo // GetRootCertsFromString parses a PEM bundle from a string and returns an x509 CertPool. func GetRootCertsFromString(caBundleString string) (*x509.CertPool, error) { if caBundleString == emptyString { - return nil, errors.New("empty string provided as caBundleString") + return nil, &EmptyArgError{Name: "caBundleString"} } rootCAPool := x509.NewCertPool() if ok := rootCAPool.AppendCertsFromPEM([]byte(caBundleString)); !ok { - return nil, errors.New("no valid certs in caBundle config string") + return nil, ErrNoCertsInConfig } return rootCAPool, nil @@ -93,11 +92,11 @@ func GetRootCertsFromString(caBundleString string) (*x509.CertPool, error) { // GetCertsFromBundle reads a PEM bundle from a file and returns a slice of x509 Certificates. func GetCertsFromBundle(certBundlePath string, fileReader Reader) ([]*x509.Certificate, error) { if certBundlePath == emptyString { - return nil, errors.New("empty string provided as certBundlePath") + return nil, &EmptyArgError{Name: "certBundlePath"} } if fileReader == nil { - return nil, errors.New("nil Reader provided") + return nil, ErrNilReader } certPEM, err := fileReader.ReadFile(certBundlePath) @@ -131,7 +130,7 @@ func GetCertsFromBundle(certBundlePath string, fileReader Reader) ([]*x509.Certi } if len(certs) == 0 { - return nil, fmt.Errorf("no valid certificates found in file %s", certBundlePath) + return nil, &NoCertsInFileError{Path: certBundlePath} } return certs, nil @@ -142,7 +141,7 @@ func GetCertsFromBundle(certBundlePath string, fileReader Reader) ([]*x509.Certi func IsPrivateKeyEncrypted(key []byte) (bool, error) { keyBlock, _ := pem.Decode(key) if keyBlock == nil { - return false, errors.New("failed to decode PEM") + return false, ErrPEMDecode } switch keyBlock.Type { @@ -152,7 +151,7 @@ func IsPrivateKeyEncrypted(key []byte) (bool, error) { _, hasDEK := keyBlock.Headers["DEK-Info"] // if encrypted, DEK-Info header exists return hasDEK, nil default: - return false, fmt.Errorf("unrecognized private key type: %s", keyBlock.Type) + return false, &UnrecognizedKeyTypeError{Type: keyBlock.Type} } } @@ -165,7 +164,7 @@ func getPassphraseIfNeeded(isEncrypted bool, pwEnvKey string, pwReader Reader) ( } if pwReader == nil { - return nil, errors.New("nil Reader provided") + return nil, ErrNilReader } pkeyEnvPw := os.Getenv(pwEnvKey) @@ -213,7 +212,7 @@ func getPassphraseIfNeeded(isEncrypted bool, pwEnvKey string, pwReader Reader) ( func ParsePrivateKey(keyPEM []byte, pwEnvKey string, pwReader Reader) (crypto.PrivateKey, error) { keyBlock, _ := pem.Decode(keyPEM) if keyBlock == nil { - return nil, errors.New("failed to decode PEM") + return nil, ErrPEMDecode } isEncrypted, _ := IsPrivateKeyEncrypted(keyPEM) @@ -255,7 +254,7 @@ func ParsePrivateKey(keyPEM []byte, pwEnvKey string, pwReader Reader) (crypto.Pr return ecKey, nil } - return nil, errors.New("unsupported key format or invalid password") + return nil, ErrUnsupportedKey } // GetKeyFromFile reads a private key from a file and parses it using ParsePrivateKey. @@ -265,16 +264,16 @@ func GetKeyFromFile( inputReader Reader, ) (crypto.PrivateKey, error) { if keyFilePath == emptyString { - return nil, errors.New("empty string provided as keyFilePath") + return nil, &EmptyArgError{Name: "keyFilePath"} } if inputReader == nil { - return nil, errors.New("nil Reader provided") + return nil, ErrNilReader } keyPEM, err := inputReader.ReadFile(keyFilePath) if err != nil { - return nil, err + return nil, fmt.Errorf("error reading private key file: %w", err) } key, err := ParsePrivateKey( diff --git a/internal/certinfo/common_handlers_test.go b/internal/certinfo/common_handlers_test.go index 3a85638..8d9cfe9 100644 --- a/internal/certinfo/common_handlers_test.go +++ b/internal/certinfo/common_handlers_test.go @@ -7,6 +7,7 @@ import ( "crypto/rand" "crypto/x509" "crypto/x509/pkix" + "errors" "math/big" "testing" "time" @@ -25,30 +26,23 @@ func TestCertinfo_GetRootCertsFromFile(t *testing.T) { inputReader, ) require.Error(t, errEmptyString) - assert.Equal(t, - "empty string provided as caBundlePath", - errEmptyString.Error(), - ) + require.ErrorIs(t, errEmptyString, ErrEmptyArg) + gotEmpty, ok := errors.AsType[*EmptyArgError](errEmptyString) + require.True(t, ok) + require.Equal(t, "caBundlePath", gotEmpty.Name) _, errNoRead := GetRootCertsFromFile( unreadableFile, mockErrReader, ) require.Error(t, errNoRead) - assert.Equal(t, - "failed to read CA bundle file: unable to read file testdata/unreadable-file.txt", - errNoRead.Error(), - ) + require.ErrorContains(t, errNoRead, "failed to read CA bundle file:") _, errWrongFile := GetRootCertsFromFile( RSACaCertKeyFile, inputReader, ) - require.Error(t, errWrongFile) - assert.Equal(t, - "unable to create CertPool from file", - errWrongFile.Error(), - ) + require.ErrorIs(t, errWrongFile, ErrCertPoolFromFile) }) t.Run("CertImportValidation", func(t *testing.T) { @@ -71,8 +65,7 @@ func TestCertinfo_GetRootCertsFromFile(t *testing.T) { RSACaCertFile, nil, ) - require.Error(t, err) - require.EqualError(t, err, "nil Reader provided") + require.ErrorIs(t, err, ErrNilReader) }) } @@ -81,17 +74,13 @@ func TestCertinfo_GetRootCertsFromString(t *testing.T) { t.Parallel() _, errEmptyString := GetRootCertsFromString(emptyString) - require.Error(t, errEmptyString) - assert.Equal(t, - "empty string provided as caBundleString", - errEmptyString.Error(), - ) + require.ErrorIs(t, errEmptyString, ErrEmptyArg) + gotEmpty, ok := errors.AsType[*EmptyArgError](errEmptyString) + require.True(t, ok) + require.Equal(t, "caBundleString", gotEmpty.Name) _, errWrongString := GetRootCertsFromString("wrong string") - require.Error(t, errWrongString) - assert.Equal(t, - "no valid certs in caBundle config string", - errWrongString.Error()) + require.ErrorIs(t, errWrongString, ErrNoCertsInConfig) }) t.Run("CertImportValidation", func(t *testing.T) { @@ -107,77 +96,102 @@ func TestCertinfo_GetRootCertsFromString(t *testing.T) { }) } -func TestCertinfo_GetCertsFromBundle(t *testing.T) { +func TestCertinfo_GetCertsFromBundle_readErrors(t *testing.T) { readErrorTests := []struct { - desc string - certPath string - reader Reader - expectedMsg string + desc string + certPath string + reader Reader + expectIs error + expectMsg string + expectPath string + expectName string }{ { - desc: "emptyString", - certPath: emptyString, - reader: inputReader, - expectedMsg: "empty string provided as certBundlePath", + desc: "emptyString", + certPath: emptyString, + reader: inputReader, + expectIs: ErrEmptyArg, + expectName: "certBundlePath", + expectMsg: "empty string provided as certBundlePath", }, { - desc: "unreadableFile", - certPath: unreadableFile, - reader: mockErrReader, - expectedMsg: "error reading certificate file: unable to read file testdata/unreadable-file.txt", + desc: "unreadableFile", + certPath: unreadableFile, + reader: mockErrReader, + expectMsg: "error reading certificate file: unable to read file testdata/unreadable-file.txt", }, { - desc: "wrong file", - certPath: RSACaCertKeyFile, - reader: inputReader, - expectedMsg: "no valid certificates found in file " + RSACaCertKeyFile, + desc: "wrong file", + certPath: RSACaCertKeyFile, + reader: inputReader, + expectIs: ErrNoCertsInFile, + expectPath: RSACaCertKeyFile, + expectMsg: "no valid certificates found in file " + RSACaCertKeyFile, }, { - desc: "nil Reader", - certPath: RSACaCertFile, - reader: nil, - expectedMsg: "nil Reader provided", + desc: "nil Reader", + certPath: RSACaCertFile, + reader: nil, + expectIs: ErrNilReader, }, { - desc: "broken cert file", - certPath: RSASamplePKCS8BrokenCertificate, - reader: inputReader, - expectedMsg: "error parsing certificate: x509: inner and outer signature algorithm identifiers don't match", + desc: "broken cert file", + certPath: RSASamplePKCS8BrokenCertificate, + reader: inputReader, + expectMsg: "error parsing certificate: x509: inner and outer signature algorithm identifiers don't match", }, } for _, tt := range readErrorTests { - t.Run("Read error "+tt.desc, func(t *testing.T) { + t.Run(tt.desc, func(t *testing.T) { t.Parallel() - _, err := GetCertsFromBundle( - tt.certPath, - tt.reader, - ) - require.Error(t, err) - assert.Equal(t, - tt.expectedMsg, - err.Error(), - ) + _, err := GetCertsFromBundle(tt.certPath, tt.reader) + requireCertinfoError(t, err, tt.expectIs, tt.expectMsg, tt.expectName, tt.expectPath) }) } +} - t.Run("CertImportValidation", func(t *testing.T) { - gotCerts, errCaString := GetCertsFromBundle( - RSACaCertFile, - inputReader, - ) - require.NoError(t, errCaString) +func TestCertinfo_GetCertsFromBundle_import(t *testing.T) { + t.Parallel() - wantCerts := []*x509.Certificate{RSACaCertParent} + gotCerts, err := GetCertsFromBundle(RSACaCertFile, inputReader) + require.NoError(t, err) - if diff := cmp.Diff(wantCerts, gotCerts); diff != "" { - t.Errorf( - "GetCertsFromBundle certs mismatch (-want +got):\n%s", - diff, - ) - } - }) + wantCerts := []*x509.Certificate{RSACaCertParent} + if diff := cmp.Diff(wantCerts, gotCerts); diff != "" { + t.Errorf("GetCertsFromBundle certs mismatch (-want +got):\n%s", diff) + } +} + +func requireCertinfoError( + t *testing.T, + err error, + expectIs error, + expectMsg, expectName, expectPath string, +) { + t.Helper() + require.Error(t, err) + + if expectIs != nil { + require.ErrorIs(t, err, expectIs) + } + + if expectName != "" { + got, ok := errors.AsType[*EmptyArgError](err) + require.True(t, ok) + require.Equal(t, expectName, got.Name) + } + + if expectPath != "" { + got, ok := errors.AsType[*NoCertsInFileError](err) + require.True(t, ok) + require.Equal(t, expectPath, got.Path) + } + + if expectMsg != "" { + require.EqualError(t, err, expectMsg) + } } //nolint:revive @@ -187,6 +201,7 @@ func TestCertinfo_GetKeyFromFile_inputReaderErrors(t *testing.T) { expectError bool keyFile string expectMsg string + expectIs error needEnv bool keyPw string }{ @@ -194,18 +209,21 @@ func TestCertinfo_GetKeyFromFile_inputReaderErrors(t *testing.T) { desc: "wrong file", expectError: true, keyFile: RSACaCertFile, + expectIs: ErrUnsupportedKey, expectMsg: "unsupported key format or invalid password", }, { desc: "emptyString", expectError: true, keyFile: emptyString, + expectIs: ErrEmptyArg, expectMsg: "empty string provided as keyFilePath", }, { desc: "No PEM encoded file", expectError: true, keyFile: sampleTextFile, + expectIs: ErrPEMDecode, expectMsg: "failed to decode PEM", }, { @@ -231,6 +249,7 @@ func TestCertinfo_GetKeyFromFile_inputReaderErrors(t *testing.T) { { desc: "Encrypted broken RSA PKCS1 key import", expectError: true, + expectIs: ErrUnsupportedKey, expectMsg: "unsupported key format or invalid password", keyFile: RSASamplePKCS1EncBrokenPrivateKey, needEnv: true, @@ -272,6 +291,7 @@ func TestCertinfo_GetKeyFromFile_inputReaderErrors(t *testing.T) { desc: "Encrypted broken ECDSA key import", expectError: true, keyFile: ECDSASampleEncBrokenPrivateKey, + expectIs: ErrUnsupportedKey, expectMsg: "unsupported key format or invalid password", needEnv: true, keyPw: samplePrivateKeyPassword, @@ -311,14 +331,8 @@ func TestCertinfo_GetKeyFromFile_inputReaderErrors(t *testing.T) { ) if tt.expectError { - require.Error(t, err) - assert.Equal(t, - tt.expectMsg, - err.Error(), - ) - } - - if !tt.expectError { + requireCertinfoError(t, err, tt.expectIs, tt.expectMsg, "", "") + } else { require.NoError(t, err) } }) @@ -333,9 +347,9 @@ func TestCertinfo_GetKeyFromFile_inputReaderErrors(t *testing.T) { mockErrReader, ) require.Error(t, errNoRead) - assert.Equal(t, - "unable to read file testdata/unreadable-file.txt", - errNoRead.Error(), + require.EqualError(t, + errNoRead, + "error reading private key file: unable to read file testdata/unreadable-file.txt", ) }) @@ -347,11 +361,7 @@ func TestCertinfo_GetKeyFromFile_inputReaderErrors(t *testing.T) { privateKeyPwEnvVar, nil, ) - require.Error(t, errNoRead) - assert.Equal(t, - "nil Reader provided", - errNoRead.Error(), - ) + require.ErrorIs(t, errNoRead, ErrNilReader) }) } @@ -407,6 +417,7 @@ func TestCertinfo_IsPrivateKeyEncrypted(t *testing.T) { require.Error(t, err) assert.EqualError(t, err, "failed to decode PEM") + require.ErrorIs(t, err, ErrPEMDecode) }) } @@ -443,11 +454,7 @@ func TestCertinfo_getPassphraseIfNeeded(t *testing.T) { privateKeyPwEnvVar, nil, ) - require.Error(t, err) - assert.EqualError(t, - err, - "nil Reader provided", - ) + require.ErrorIs(t, err, ErrNilReader) }) t.Run("pw read success", func(t *testing.T) { @@ -474,14 +481,14 @@ func TestCertinfo_certMatchPrivateKey_matchFalse(t *testing.T) { cert *x509.Certificate key crypto.PrivateKey expectErr bool - expectMsg string + expectIs error }{ { desc: "uncomplete cert", cert: &incompleteCert, key: RSASampleCertKey, expectErr: true, - expectMsg: "unsupported public key type in certificate", + expectIs: ErrUnsupportedPublicKey, }, { @@ -518,8 +525,7 @@ func TestCertinfo_certMatchPrivateKey_matchFalse(t *testing.T) { } if tt.expectErr { - require.Error(t, err) - require.EqualError(t, err, tt.expectMsg) + require.ErrorIs(t, err, tt.expectIs) } }) } diff --git a/internal/certinfo/errors.go b/internal/certinfo/errors.go new file mode 100644 index 0000000..24e84a5 --- /dev/null +++ b/internal/certinfo/errors.go @@ -0,0 +1,68 @@ +package certinfo + +import ( + "errors" + "fmt" +) + +// Package-level sentinels for stable certinfo failure conditions. +// Match them with errors.Is after wrapping; Error() strings stay human-facing. +var ( + ErrNilReader = errors.New("nil Reader provided") + ErrPEMDecode = errors.New("failed to decode PEM") + ErrCertPoolFromFile = errors.New("unable to create CertPool from file") + ErrNoCertsInConfig = errors.New("no valid certs in caBundle config string") + ErrUnsupportedKey = errors.New("unsupported key format or invalid password") + ErrUnsupportedPublicKey = errors.New("unsupported public key type in certificate") + ErrEmptyArg = errors.New("empty string provided as argument") + ErrNoCertsInFile = errors.New("no valid certificates found in file") + ErrUnrecognizedKeyType = errors.New("unrecognized private key type") +) + +// EmptyArgError is returned when a required string argument is empty. +// errors.Is(err, ErrEmptyArg) is true. +type EmptyArgError struct { + Name string +} + +// Error returns a message naming the empty argument. +func (e *EmptyArgError) Error() string { + return fmt.Sprintf("empty string provided as %s", e.Name) +} + +// Is reports whether target is ErrEmptyArg. +func (*EmptyArgError) Is(target error) bool { + return target == ErrEmptyArg +} + +// NoCertsInFileError is returned when a PEM file yields no certificates. +// errors.Is(err, ErrNoCertsInFile) is true. +type NoCertsInFileError struct { + Path string +} + +// Error returns a message including the PEM file path. +func (e *NoCertsInFileError) Error() string { + return fmt.Sprintf("no valid certificates found in file %s", e.Path) +} + +// Is reports whether target is ErrNoCertsInFile. +func (*NoCertsInFileError) Is(target error) bool { + return target == ErrNoCertsInFile +} + +// UnrecognizedKeyTypeError is returned for an unknown PEM private-key type. +// errors.Is(err, ErrUnrecognizedKeyType) is true. +type UnrecognizedKeyTypeError struct { + Type string +} + +// Error returns a message including the unrecognized PEM type. +func (e *UnrecognizedKeyTypeError) Error() string { + return fmt.Sprintf("unrecognized private key type: %s", e.Type) +} + +// Is reports whether target is ErrUnrecognizedKeyType. +func (*UnrecognizedKeyTypeError) Is(target error) bool { + return target == ErrUnrecognizedKeyType +} diff --git a/internal/certinfo/errors_test.go b/internal/certinfo/errors_test.go new file mode 100644 index 0000000..becd32f --- /dev/null +++ b/internal/certinfo/errors_test.go @@ -0,0 +1,96 @@ +package certinfo + +import ( + "errors" + "fmt" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCertinfo_errorSentinels_Is(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + err error + target error + }{ + { + name: "EmptyArgError", + err: &EmptyArgError{Name: "caBundlePath"}, + target: ErrEmptyArg, + }, + { + name: "EmptyArgError wrapped", + err: fmt.Errorf("load: %w", &EmptyArgError{Name: "keyFilePath"}), + target: ErrEmptyArg, + }, + { + name: "NoCertsInFileError", + err: &NoCertsInFileError{Path: "bundle.pem"}, + target: ErrNoCertsInFile, + }, + { + name: "UnrecognizedKeyTypeError", + err: &UnrecognizedKeyTypeError{Type: "FOO"}, + target: ErrUnrecognizedKeyType, + }, + { + name: "ErrNilReader wrapped", + err: fmt.Errorf("op: %w", ErrNilReader), + target: ErrNilReader, + }, + { + name: "ErrPEMDecode", + err: ErrPEMDecode, + target: ErrPEMDecode, + }, + { + name: "ErrCertPoolFromFile", + err: ErrCertPoolFromFile, + target: ErrCertPoolFromFile, + }, + { + name: "ErrNoCertsInConfig", + err: ErrNoCertsInConfig, + target: ErrNoCertsInConfig, + }, + { + name: "ErrUnsupportedKey", + err: ErrUnsupportedKey, + target: ErrUnsupportedKey, + }, + { + name: "ErrUnsupportedPublicKey", + err: ErrUnsupportedPublicKey, + target: ErrUnsupportedPublicKey, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + require.ErrorIs(t, tt.err, tt.target) + }) + } +} + +func TestCertinfo_errorTypes_AsType(t *testing.T) { + t.Parallel() + + empty := fmt.Errorf("wrap: %w", &EmptyArgError{Name: "certBundlePath"}) + gotEmpty, ok := errors.AsType[*EmptyArgError](empty) + require.True(t, ok) + require.Equal(t, "certBundlePath", gotEmpty.Name) + + noCerts := fmt.Errorf("wrap: %w", &NoCertsInFileError{Path: "a.pem"}) + gotNoCerts, ok := errors.AsType[*NoCertsInFileError](noCerts) + require.True(t, ok) + require.Equal(t, "a.pem", gotNoCerts.Path) + + keyType := fmt.Errorf("wrap: %w", &UnrecognizedKeyTypeError{Type: "CERTIFICATE"}) + gotKeyType, ok := errors.AsType[*UnrecognizedKeyTypeError](keyType) + require.True(t, ok) + require.Equal(t, "CERTIFICATE", gotKeyType.Type) +} diff --git a/internal/cmd/certinfo.go b/internal/cmd/certinfo.go index 5820c74..5b59f19 100644 --- a/internal/cmd/certinfo.go +++ b/internal/cmd/certinfo.go @@ -10,6 +10,7 @@ import ( "github.com/spf13/cobra" "github.com/spf13/viper" "github.com/xenos76/https-wrench/internal/certinfo" + "github.com/xenos76/https-wrench/internal/errdisp" ) var ( @@ -84,16 +85,16 @@ Examples: certinfoCfg, err := certinfo.New() if err != nil { - cmd.Printf("Error creating new Certinfo config: %s", err) + cmd.Printf("Error creating new Certinfo config: %s", errdisp.FormatCause(err)) return } if err = certinfoCfg.SetCaPoolFromFile(caBundleValue, fileReader); err != nil { - cmd.Printf("Error importing CA Certificate bundle from file: %s", err) + cmd.Printf("Error importing CA Certificate bundle from file: %s", errdisp.FormatCause(err)) } if err = certinfoCfg.SetCertsFromFile(certBundleValue, fileReader); err != nil { - cmd.Printf("Error importing Certificate bundle from file: %s", err) + cmd.Printf("Error importing Certificate bundle from file: %s", errdisp.FormatCause(err)) } certinfoCfg.SetTLSInsecure(tlsInsecure).SetTLSServerName(tlsServerName).SetTLSInfoRequested(tlsInfo) @@ -102,7 +103,7 @@ Examples: // before being able to ask details about the certificate we want to a // webserver using self-signed and valid certificates if err = certinfoCfg.SetTLSEndpoint(context.Background(), tlsEndpoint); err != nil { - cmd.Printf("Error setting TLS endpoint: %s", err) + cmd.Printf("Error setting TLS endpoint: %s", errdisp.FormatCause(err)) return } @@ -111,12 +112,12 @@ Examples: keyPwEnvVar, fileReader, ); err != nil { - cmd.Printf("Error importing key from file: %s", err) + cmd.Printf("Error importing key from file: %s", errdisp.FormatCause(err)) } // dump.Print(certinfoCfg) if err = certinfoCfg.PrintData(context.Background(), cmd.OutOrStdout()); err != nil { - cmd.Printf("error printing Certinfo data: %s", err) + cmd.Printf("error printing Certinfo data: %s", errdisp.FormatCause(err)) } }, } diff --git a/internal/cmd/jwtinfo_test.go b/internal/cmd/jwtinfo_test.go index 51fe0dc..c067a25 100644 --- a/internal/cmd/jwtinfo_test.go +++ b/internal/cmd/jwtinfo_test.go @@ -64,7 +64,7 @@ func TestJwtinfoCmd_Success(t *testing.T) { token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9." + "eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2M" + "jM5MDIyLCJleHAiOjE1MTYyNDkwMjJ9.c2lnbmF0dXJl" - w.Write([]byte(`{"access_token": "` + token + `"}`)) + _, _ = w.Write([]byte(`{"access_token": "` + token + `"}`)) })) defer ts.Close() diff --git a/internal/cmd/requests.go b/internal/cmd/requests.go index 414df0c..b8f901f 100644 --- a/internal/cmd/requests.go +++ b/internal/cmd/requests.go @@ -13,6 +13,7 @@ import ( "github.com/gookit/goutil/dump" "github.com/spf13/cobra" "github.com/spf13/viper" + "github.com/xenos76/https-wrench/internal/errdisp" "github.com/xenos76/https-wrench/internal/requests" ) @@ -89,16 +90,16 @@ Examples: SetRequests(cfg.Requests) if err := requestsCfg.SetCaPoolFromYAML(cfg.CaBundle); err != nil { - cmd.Print(err) + cmd.Print(errdisp.Format(err)) } if err := requestsCfg.SetCaPoolFromFile(caBundlePath, fileReader); err != nil { - cmd.Print(err) + cmd.Print(errdisp.Format(err)) } responseMap, err := requests.HandleRequests(context.Background(), cmd.OutOrStdout(), requestsCfg) if err != nil { - cmd.Print(err) + cmd.Print(errdisp.Format(err)) } if cfg.Debug { diff --git a/internal/errdisp/errdisp.go b/internal/errdisp/errdisp.go new file mode 100644 index 0000000..d80c0ae --- /dev/null +++ b/internal/errdisp/errdisp.go @@ -0,0 +1,120 @@ +/* +Copyright © 2025 Zeno Belli xeno@os76.xyz +*/ + +// Package errdisp formats errors for CLI and MCP user boundaries. +// It holds no sentinels; domain identity stays in packages such as certinfo. +package errdisp + +import ( + "errors" + "strings" + + "github.com/xenos76/https-wrench/internal/certinfo" +) + +// Cause returns the deepest single-cause unwrap of err. +// Multi-cause Join chains are left as-is (no invented multi-cause UX). +func Cause(err error) error { + for err != nil { + u := errors.Unwrap(err) + if u == nil { + return err + } + + err = u + } + + return err +} + +// FormatCause returns a short message for callers that already print an +// operation prefix. Prefer certinfo domain leaves via Is/As; otherwise the +// deepest cause. +func FormatCause(err error) string { + if err == nil { + return "" + } + + if msg, ok := domainLeaf(err); ok { + return msg + } + + return Cause(err).Error() +} + +// Format returns a user-facing message when the caller has no operation prefix. +// Prefer certinfo domain leaves via Is/As; otherwise top wrap label + deepest +// cause, skipping intermediate layers. +func Format(err error) string { + if err == nil { + return "" + } + + if msg, ok := domainLeaf(err); ok { + return msg + } + + if errors.Unwrap(err) == nil { + return err.Error() + } + + cause := Cause(err) + label := topLabel(err) + + if label == "" || label == cause.Error() { + return cause.Error() + } + + return label + ": " + cause.Error() +} + +// domainLeaf returns a certinfo leaf message when err matches a known domain failure. +func domainLeaf(err error) (string, bool) { + if empty, ok := errors.AsType[*certinfo.EmptyArgError](err); ok { + return empty.Error(), true + } + + if noCerts, ok := errors.AsType[*certinfo.NoCertsInFileError](err); ok { + return noCerts.Error(), true + } + + if keyType, ok := errors.AsType[*certinfo.UnrecognizedKeyTypeError](err); ok { + return keyType.Error(), true + } + + for _, s := range []error{ + certinfo.ErrNilReader, + certinfo.ErrPEMDecode, + certinfo.ErrCertPoolFromFile, + certinfo.ErrNoCertsInConfig, + certinfo.ErrUnsupportedKey, + certinfo.ErrUnsupportedPublicKey, + certinfo.ErrEmptyArg, + certinfo.ErrNoCertsInFile, + certinfo.ErrUnrecognizedKeyType, + } { + if errors.Is(err, s) { + return s.Error(), true + } + } + + return "", false +} + +// topLabel returns the outermost wrap text without the unwrapped suffix. +func topLabel(err error) string { + u := errors.Unwrap(err) + if u == nil { + return err.Error() + } + + full := err.Error() + + suffix := ": " + u.Error() + if after, ok := strings.CutSuffix(full, suffix); ok { + return after + } + + return full +} diff --git a/internal/errdisp/errdisp_test.go b/internal/errdisp/errdisp_test.go new file mode 100644 index 0000000..0df15bb --- /dev/null +++ b/internal/errdisp/errdisp_test.go @@ -0,0 +1,90 @@ +/* +Copyright © 2025 Zeno Belli xeno@os76.xyz +*/ + +package errdisp + +import ( + "errors" + "fmt" + "testing" + + "github.com/stretchr/testify/require" + "github.com/xenos76/https-wrench/internal/certinfo" +) + +func TestCause(t *testing.T) { + t.Parallel() + + require.NoError(t, Cause(nil)) + + leaf := errors.New("leaf") + require.Equal(t, leaf, Cause(leaf)) + + wrapped := fmt.Errorf("mid: %w", fmt.Errorf("inner: %w", leaf)) + require.Equal(t, leaf, Cause(wrapped)) +} + +func TestFormatCause(t *testing.T) { + t.Parallel() + + require.Empty(t, FormatCause(nil)) + + t.Run("os cause under read wrap", func(t *testing.T) { + t.Parallel() + + err := fmt.Errorf("failed to read CA bundle file: %w", errors.New("open x: no such file or directory")) + require.Equal(t, "open x: no such file or directory", FormatCause(err)) + }) + + t.Run("PEM sentinel", func(t *testing.T) { + t.Parallel() + + err := fmt.Errorf("error reading private key file: %w", certinfo.ErrPEMDecode) + require.Equal(t, certinfo.ErrPEMDecode.Error(), FormatCause(err)) + }) + + t.Run("typed no certs", func(t *testing.T) { + t.Parallel() + + err := fmt.Errorf("wrap: %w", &certinfo.NoCertsInFileError{Path: "a.pem"}) + require.Equal(t, "no valid certificates found in file a.pem", FormatCause(err)) + }) + + t.Run("typed empty arg", func(t *testing.T) { + t.Parallel() + + err := fmt.Errorf("wrap: %w", &certinfo.EmptyArgError{Name: "caBundlePath"}) + require.Equal(t, "empty string provided as caBundlePath", FormatCause(err)) + }) +} + +func TestFormat(t *testing.T) { + t.Parallel() + + require.Empty(t, Format(nil)) + + t.Run("leaf only for sentinel", func(t *testing.T) { + t.Parallel() + + err := fmt.Errorf("unable to create CA Certs Pool from YAML: %w", certinfo.ErrNoCertsInConfig) + require.Equal(t, certinfo.ErrNoCertsInConfig.Error(), Format(err)) + }) + + t.Run("top context and cause skips middle", func(t *testing.T) { + t.Parallel() + + leaf := errors.New("open x: no such file or directory") + err := fmt.Errorf( + "unable to get endpoint certificates: %w", + fmt.Errorf("TLS handshake failed: %w", leaf), + ) + require.Equal(t, "unable to get endpoint certificates: open x: no such file or directory", Format(err)) + }) + + t.Run("bare leaf", func(t *testing.T) { + t.Parallel() + + require.Equal(t, "boom", Format(errors.New("boom"))) + }) +} diff --git a/internal/jwks/jwks_test.go b/internal/jwks/jwks_test.go index 2ce5034..b35642c 100644 --- a/internal/jwks/jwks_test.go +++ b/internal/jwks/jwks_test.go @@ -32,7 +32,7 @@ func TestGenerateJWKS_Success(t *testing.T) { require.NoError(t, err) err = pem.Encode(file, block) require.NoError(t, err) - file.Close() + require.NoError(t, file.Close()) return path } diff --git a/internal/jwtinfo/jwtinfo_refresh_test.go b/internal/jwtinfo/jwtinfo_refresh_test.go index b1169fd..acfe6ad 100644 --- a/internal/jwtinfo/jwtinfo_refresh_test.go +++ b/internal/jwtinfo/jwtinfo_refresh_test.go @@ -140,7 +140,7 @@ func TestJwtTokenData_WriteTokenToFile(t *testing.T) { tempFile, err := os.CreateTemp("", "token-test-*") require.NoError(t, err) - tempFile.Close() + require.NoError(t, tempFile.Close()) defer os.Remove(tempFile.Name()) diff --git a/internal/mcp/tools_exec.go b/internal/mcp/tools_exec.go index 907648b..45de98e 100644 --- a/internal/mcp/tools_exec.go +++ b/internal/mcp/tools_exec.go @@ -15,6 +15,7 @@ import ( sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/spf13/viper" "github.com/xenos76/https-wrench/internal/certinfo" + "github.com/xenos76/https-wrench/internal/errdisp" "github.com/xenos76/https-wrench/internal/jwks" "github.com/xenos76/https-wrench/internal/jwtinfo" "github.com/xenos76/https-wrench/internal/requests" @@ -102,6 +103,7 @@ func registerExecTools(server *sdkmcp.Server) { }, generateJWKSHandler) } +// runRequestsHandler executes the run_requests MCP tool. func runRequestsHandler( ctx context.Context, _ *sdkmcp.CallToolRequest, @@ -112,12 +114,13 @@ func runRequestsHandler( out, err := executeRunRequests(ctx, input) if err != nil { - return nil, execToolOutput{Error: err.Error()}, nil + return nil, execToolOutput{Error: errdisp.Format(err)}, nil } return nil, out, nil } +// certinfoHandler executes the certinfo MCP tool. func certinfoHandler( ctx context.Context, _ *sdkmcp.CallToolRequest, @@ -128,12 +131,13 @@ func certinfoHandler( out, err := executeCertinfo(ctx, input) if err != nil { - return nil, execToolOutput{Error: err.Error()}, nil + return nil, execToolOutput{Error: errdisp.Format(err)}, nil } return nil, out, nil } +// jwtinfoHandler executes the jwtinfo MCP tool. func jwtinfoHandler( ctx context.Context, _ *sdkmcp.CallToolRequest, @@ -144,12 +148,13 @@ func jwtinfoHandler( out, err := executeJwtinfo(ctx, input) if err != nil { - return nil, execToolOutput{Error: err.Error()}, nil + return nil, execToolOutput{Error: errdisp.Format(err)}, nil } return nil, out, nil } +// generateJWKSHandler executes the generate_jwks MCP tool. func generateJWKSHandler( ctx context.Context, _ *sdkmcp.CallToolRequest, @@ -160,7 +165,7 @@ func generateJWKSHandler( out, err := executeGenerateJWKS(ctx, input) if err != nil { - return nil, execToolOutput{Error: err.Error()}, nil + return nil, execToolOutput{Error: errdisp.Format(err)}, nil } return nil, out, nil diff --git a/internal/requests/requests.go b/internal/requests/requests.go index 50713c9..aa054e3 100644 --- a/internal/requests/requests.go +++ b/internal/requests/requests.go @@ -214,7 +214,7 @@ func (r *RequestsMetaConfig) SetCaPoolFromYAML(s string) error { if s != "" { certsPool, err := certinfo.GetRootCertsFromString(s) if err != nil { - return errors.New("unable to create CA Certs Pool from YAML") + return fmt.Errorf("unable to create CA Certs Pool from YAML: %w", err) } r.CACertsPool = certsPool diff --git a/internal/requests/requests_handlers.go b/internal/requests/requests_handlers.go index ec5e960..a37b6c5 100644 --- a/internal/requests/requests_handlers.go +++ b/internal/requests/requests_handlers.go @@ -279,7 +279,7 @@ func (rd *ResponseData) ImportResponseBody() { err := json.Indent(&prettyJSON, body, "", " ") if err != nil { - prettyJSON.Write(body) + _, _ = prettyJSON.Write(body) } code = prettyJSON.String() diff --git a/internal/requests/requests_test.go b/internal/requests/requests_test.go index 9c99518..62f7033 100644 --- a/internal/requests/requests_test.go +++ b/internal/requests/requests_test.go @@ -182,6 +182,7 @@ func TestRequestsMetaConfig_SetCaPoolFromYAML_Error(t *testing.T) { err := rmc.SetCaPoolFromYAML("invalid cert data") require.Error(t, err) require.ErrorContains(t, err, "unable to create CA Certs Pool from YAML") + require.ErrorIs(t, err, certinfo.ErrNoCertsInConfig) }) }