Skip to content

Commit 7e803bb

Browse files
committed
fix(venom): plugin loading, TLS paths, exec hardening, SSH timeout, HTML sanitization
- venom: load .so plugins only from --lib-dir (no auto-load from workdir); validate plugin name against [A-Za-z0-9_-] - http: reject absolute paths for tls_root_ca/tls_client_cert/tls_client_key; detect inline PEM via "-----BEGIN" marker; fail explicitly when file is missing - exec: fix Windows .ps1 rename (create temp script with final name); create temp script atomically with O_EXCL and 0o700; document stdout/stderr synchronization - ssh: add configurable `timeout` field (default 30s); restrict privatekey absolute paths to $HOME, resolve relative paths under workdir - html report: sanitize markdown rendering with DOMPurify BREAKING CHANGE: .so plugins now require --lib-dir; HTTP TLS file paths must be relative to workdir; SSH privatekey absolute paths must be under $HOME. Signed-off-by: Yvonnick Esnault <yvonnick.esnault@ovhcloud.com>
1 parent 6ccc46c commit 7e803bb

13 files changed

Lines changed: 205 additions & 108 deletions

File tree

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,8 @@ Notice the variable `alljson`. All variables declared in output are automaticall
419419
Venom will load user-defined executors from the directory `lib/` relative to the testsuite path. You can add executor source paths using the flag `--lib-dir`.
420420
Note that all folders listed with `--lib-dir` will be scanned recursively to find `.yml` files as user executors.
421421

422+
Compiled Go plugins (`.so`) are **only** loaded from `--lib-dir`; they are no longer auto-discovered from `<workdir>/lib/`. The plugin name (the YAML `type:` field) must match `[A-Za-z0-9_-]+`.
423+
422424
The user defined executors work with templating, you can check the templating result in `venom.log`. In this file, if you see an error such as `error converting YAML to JSON: yaml: line 14: found unexpected end of stream`, you probably need to adjust indentation with the templating function `indent`.
423425

424426
Example:

executors/exec/exec.go

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@ package exec
22

33
import (
44
"context"
5+
"crypto/rand"
6+
"encoding/hex"
57
"fmt"
68
"os"
79
"os/exec"
10+
"path/filepath"
811
"runtime"
912
"strconv"
1013
"strings"
@@ -97,48 +100,43 @@ func (Executor) Run(ctx context.Context, step venom.TestStep) (interface{}, erro
97100
opts = append(opts, "-ExecutionPolicy", "Bypass", "-Command")
98101
}
99102

100-
// Create a tmp file
101-
tmpscript, err := os.CreateTemp(os.TempDir(), "venom-")
103+
// Create the tmp script file atomically with the right permissions
104+
// (O_EXCL avoids races, 0o700 keeps the script private to the user).
105+
nameBytes := make([]byte, 16)
106+
if _, err := rand.Read(nameBytes); err != nil {
107+
return nil, fmt.Errorf("cannot generate tmp name: %s", err)
108+
}
109+
baseName := "venom-" + hex.EncodeToString(nameBytes)
110+
if runtime.GOOS == "windows" {
111+
baseName += ".PS1"
112+
}
113+
scriptPath := filepath.Join(os.TempDir(), baseName)
114+
tmpscript, err := os.OpenFile(scriptPath, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o700)
102115
if err != nil {
103116
return nil, fmt.Errorf("cannot create tmp file: %s", err)
104117
}
105118

106-
// Put script in file
107-
venom.Debug(ctx, "work with tmp file %s", tmpscript.Name())
119+
venom.Debug(ctx, "work with tmp file %s", scriptPath)
108120
n, err := tmpscript.Write([]byte(scriptContent))
109121
if err != nil || n != len(scriptContent) {
122+
tmpscript.Close()
123+
os.Remove(scriptPath)
110124
if err != nil {
111125
return nil, fmt.Errorf("cannot write script: %s", err)
112126
}
113127
return nil, fmt.Errorf("cannot write all script: %d/%d", n, len(scriptContent))
114128
}
115-
116-
oldPath := tmpscript.Name()
117129
tmpscript.Close()
118-
var scriptPath string
130+
119131
if runtime.GOOS == "windows" {
120-
// Remove all .txt Extensions, there is not always a .txt extension
121-
newPath := strings.ReplaceAll(oldPath, ".txt", "")
122-
// and add .PS1 extension
123-
newPath += ".PS1"
124-
if err := os.Rename(oldPath, newPath); err != nil {
125-
return nil, fmt.Errorf("cannot rename script to add powershell extension, aborting")
126-
}
127132
// This aims to stop a the very first error and return the right exit code
128-
psCommand := fmt.Sprintf("& { $ErrorActionPreference='Stop'; & %s ;exit $LastExitCode}", newPath)
129-
scriptPath = newPath
133+
psCommand := fmt.Sprintf("& { $ErrorActionPreference='Stop'; & %s ;exit $LastExitCode}", scriptPath)
130134
opts = append(opts, psCommand)
131135
} else {
132-
scriptPath = oldPath
133136
opts = append(opts, scriptPath)
134137
}
135138
defer os.Remove(scriptPath)
136139

137-
// Chmod file
138-
if err := os.Chmod(scriptPath, 0o700); err != nil {
139-
return nil, fmt.Errorf("cannot chmod script %s: %s", scriptPath, err)
140-
}
141-
142140
command = shell
143141
}
144142

@@ -161,6 +159,12 @@ func (Executor) Run(ctx context.Context, step venom.TestStep) (interface{}, erro
161159
}
162160

163161
result := Result{}
162+
163+
// The two goroutines below are the only writers of result.Systemout and
164+
// result.Systemerr. The parent reads these fields only after <-outchan
165+
// and <-errchan have unblocked, which happens-after their respective
166+
// close. Do not access result.Systemout / result.Systemerr from any
167+
// other goroutine while the command is running.
164168
outchan := make(chan bool)
165169

166170
go func() {
@@ -194,7 +198,7 @@ func (Executor) Run(ctx context.Context, step venom.TestStep) (interface{}, erro
194198
if n > 0 {
195199
chunk := buf[:n]
196200
sb.Write(chunk)
197-
venom.Debug(ctx, venom.HideSensitive(ctx, string(chunk)))
201+
venom.Debug(ctx, "%s", venom.HideSensitive(ctx, string(chunk)))
198202
}
199203
if err != nil {
200204
break

executors/http/README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ In your yaml file, you can use:
2323
- no_follow_redirect (optional): indicates that you don't want to follow Location if server returns a Redirect (301/302/...)
2424
- skip_body: skip the body and bodyjson result
2525
- skip_headers: skip the headers result
26-
- tls_client_cert (optional): a chain of certificates to identify the caller, first certificate in the chain is considered as the leaf, followed by intermediates. Setting it enables mutual TLS authentication. Set the PEM content or the path to the PEM file.
27-
- tls_client_key (optional): private key corresponding to the certificate. Set the PEM content or the path to the PEM file.
28-
- tls_root_ca (optional): defines additional root CAs to perform the call. Can contain multiple CAs concatenated together. Set the PEM content or the path to the PEM file.
26+
- tls_client_cert (optional): a chain of certificates to identify the caller, first certificate in the chain is considered as the leaf, followed by intermediates. Setting it enables mutual TLS authentication. Provide either inline PEM content (must contain "-----BEGIN") or a path relative to the testsuite workdir. Absolute paths are rejected.
27+
- tls_client_key (optional): private key corresponding to the certificate. Provide either inline PEM content or a path relative to the testsuite workdir.
28+
- tls_root_ca (optional): additional root CAs used for the call. Can contain multiple CAs concatenated together. Provide either inline PEM content or a path relative to the testsuite workdir.
2929

3030
```
3131

executors/http/http.go

Lines changed: 31 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,25 @@ func isBodyJSONSupported(resp *http.Response) bool {
428428
return strings.Contains(contentType, "application/json") || strings.HasSuffix(contentType, "+json")
429429
}
430430

431+
// loadPEMOrFile decides whether the given value is inline PEM content or a
432+
// path to a file under the testsuite workdir. Inline PEM (recognised by the
433+
// "-----BEGIN" marker) is returned as-is. Otherwise the value is treated as
434+
// a relative path, sanitised against escapes, and read from disk.
435+
func loadPEMOrFile(workdir, value, fieldName string) ([]byte, error) {
436+
if strings.Contains(value, "-----BEGIN") {
437+
return []byte(value), nil
438+
}
439+
resolved, err := venom.ResolveWorkdirPath(workdir, value)
440+
if err != nil {
441+
return nil, fmt.Errorf("invalid %s path: %w", fieldName, err)
442+
}
443+
data, err := os.ReadFile(resolved)
444+
if err != nil {
445+
return nil, fmt.Errorf("unable to read %s from file %s: %w", fieldName, resolved, err)
446+
}
447+
return data, nil
448+
}
449+
431450
func (e Executor) TLSOptions(ctx context.Context) ([]func(*http.Transport) error, error) {
432451
var opts []func(*http.Transport) error
433452

@@ -438,51 +457,28 @@ func (e Executor) TLSOptions(ctx context.Context) ([]func(*http.Transport) error
438457
workdir := venom.StringVarFromCtx(ctx, "venom.testsuite.workdir")
439458

440459
if e.TLSRootCA != "" {
441-
TLSRootCAFilepath := e.TLSRootCA
442-
if !filepath.IsAbs(e.TLSRootCA) {
443-
TLSRootCAFilepath = filepath.Join(workdir, e.TLSRootCA)
444-
}
445-
var TLSRootCA []byte
446-
if _, err := os.Stat(TLSRootCAFilepath); err == nil {
447-
TLSRootCA, err = os.ReadFile(TLSRootCAFilepath)
448-
if err != nil {
449-
return nil, fmt.Errorf("unable to read TLSRootCA from file %s", TLSRootCAFilepath)
450-
}
451-
} else {
452-
TLSRootCA = []byte(e.TLSRootCA)
460+
data, err := loadPEMOrFile(workdir, e.TLSRootCA, "TLSRootCA")
461+
if err != nil {
462+
return nil, err
453463
}
454-
opts = append(opts, WithTLSRootCA(ctx, TLSRootCA))
464+
opts = append(opts, WithTLSRootCA(ctx, data))
455465
}
456466

457467
var TLSClientCert, TLSClientKey []byte
458468
if e.TLSClientCert != "" {
459-
TLSClientCertFilepath := e.TLSClientCert
460-
if !filepath.IsAbs(e.TLSClientCert) {
461-
TLSClientCertFilepath = filepath.Join(workdir, e.TLSClientCert)
462-
}
463-
if _, err := os.Stat(TLSClientCertFilepath); err == nil {
464-
TLSClientCert, err = os.ReadFile(TLSClientCertFilepath)
465-
if err != nil {
466-
return nil, fmt.Errorf("unable to read TLSClientCert from file %s", TLSClientCertFilepath)
467-
}
468-
} else {
469-
TLSClientCert = []byte(e.TLSClientCert)
469+
data, err := loadPEMOrFile(workdir, e.TLSClientCert, "TLSClientCert")
470+
if err != nil {
471+
return nil, err
470472
}
473+
TLSClientCert = data
471474
}
472475

473476
if e.TLSClientKey != "" {
474-
TLSClientKeyFilepath := e.TLSClientKey
475-
if !filepath.IsAbs(e.TLSClientKey) {
476-
TLSClientKeyFilepath = filepath.Join(workdir, e.TLSClientKey)
477-
}
478-
if _, err := os.Stat(TLSClientKeyFilepath); err == nil {
479-
TLSClientKey, err = os.ReadFile(TLSClientKeyFilepath)
480-
if err != nil {
481-
return nil, fmt.Errorf("unable to read TLSClientKey from file %s", TLSClientKeyFilepath)
482-
}
483-
} else {
484-
TLSClientKey = []byte(e.TLSClientKey)
477+
data, err := loadPEMOrFile(workdir, e.TLSClientKey, "TLSClientKey")
478+
if err != nil {
479+
return nil, err
485480
}
481+
TLSClientKey = data
486482
}
487483

488484
if len(TLSClientCert) > 0 && len(TLSClientKey) > 0 {

executors/http/http_test.go

Lines changed: 42 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,48 +16,61 @@ import (
1616
"github.com/ovh/venom"
1717
)
1818

19-
func generateClientFile(t *testing.T) (string, string) {
20-
TLSClientKey, err := os.CreateTemp(os.TempDir(), "TLSClientKey.*.key")
21-
require.NoError(t, err)
22-
TLSClientKeyFileName := TLSClientKey.Name()
23-
t.Logf("generating file %q", TLSClientKeyFileName)
24-
cmd := exec.Command("openssl", "genrsa", "-out", TLSClientKeyFileName, "2048")
19+
// generateClientFile creates a TLS key and self-signed certificate inside dir.
20+
// Returns the file names (relative to dir) so tests can resolve them through
21+
// venom.ResolveWorkdirPath.
22+
func generateClientFile(t *testing.T, dir string) (string, string) {
23+
keyPath := "TLSClientKey.key"
24+
certPath := "TLSClientCert.crt"
25+
absKey := dir + "/" + keyPath
26+
absCert := dir + "/" + certPath
27+
28+
t.Logf("generating file %q", absKey)
29+
cmd := exec.Command("openssl", "genrsa", "-out", absKey, "2048")
2530
output, err := cmd.CombinedOutput()
2631
t.Log(string(output))
2732
require.NoError(t, err)
2833

29-
TLSClientCert, err := os.CreateTemp(os.TempDir(), "TLSClientCert.*.crt")
30-
require.NoError(t, err)
31-
TLSClientCertFilename := TLSClientCert.Name()
32-
t.Logf("generating file %q", TLSClientCertFilename)
33-
cmd = exec.Command("openssl", "req", "-batch", "-subj", "/C=GB/ST=Yorks/L=York/O=MyCompany Ltd./OU=IT/CN=mysubdomain.mydomain.com", "-new", "-x509", "-sha256", "-key", TLSClientKeyFileName, "-out", TLSClientCertFilename, "-days", "365")
34+
t.Logf("generating file %q", absCert)
35+
cmd = exec.Command("openssl", "req", "-batch", "-subj", "/C=GB/ST=Yorks/L=York/O=MyCompany Ltd./OU=IT/CN=mysubdomain.mydomain.com", "-new", "-x509", "-sha256", "-key", absKey, "-out", absCert, "-days", "365")
3436
output, err = cmd.CombinedOutput()
3537
t.Log(string(output))
3638
require.NoError(t, err)
3739

38-
return TLSClientKeyFileName, TLSClientCertFilename
40+
return keyPath, certPath
41+
}
42+
43+
func ctxWithWorkdir(workdir string) context.Context {
44+
return context.WithValue(context.Background(), venom.ContextKey("var.venom.testsuite.workdir"), workdir)
3945
}
4046

4147
func TestExecutor_TLSOptions_From_File(t *testing.T) {
42-
TLSClientKeyFileName, TLSClientCertFilename := generateClientFile(t)
48+
workdir := t.TempDir()
49+
keyName, certName := generateClientFile(t, workdir)
50+
51+
rootCA, err := os.ReadFile("../../tests/http/tls/digicert-root-ca.crt")
52+
require.NoError(t, err)
53+
rootCAPath := "digicert-root-ca.crt"
54+
require.NoError(t, os.WriteFile(workdir+"/"+rootCAPath, rootCA, 0o600))
4355

4456
e := Executor{
4557
IgnoreVerifySSL: true,
46-
TLSClientCert: TLSClientCertFilename,
47-
TLSClientKey: TLSClientKeyFileName,
48-
TLSRootCA: "../../tests/http/tls/digicert-root-ca.crt",
58+
TLSClientCert: certName,
59+
TLSClientKey: keyName,
60+
TLSRootCA: rootCAPath,
4961
}
50-
opts, err := e.TLSOptions(context.Background())
62+
opts, err := e.TLSOptions(ctxWithWorkdir(workdir))
5163
require.NoError(t, err)
5264
require.Len(t, opts, 3)
5365
}
5466

5567
func TestExecutor_TLSOptions_From_String(t *testing.T) {
56-
TLSClientKeyFileName, TLSClientCertFilename := generateClientFile(t)
68+
workdir := t.TempDir()
69+
keyName, certName := generateClientFile(t, workdir)
5770

58-
TLSClientCert, err := os.ReadFile(TLSClientCertFilename)
71+
TLSClientCert, err := os.ReadFile(workdir + "/" + certName)
5972
require.NoError(t, err)
60-
TLSClientKey, err := os.ReadFile(TLSClientKeyFileName)
73+
TLSClientKey, err := os.ReadFile(workdir + "/" + keyName)
6174
require.NoError(t, err)
6275
TLSRootCA, err := os.ReadFile("../../tests/http/tls/digicert-root-ca.crt")
6376
require.NoError(t, err)
@@ -66,11 +79,19 @@ func TestExecutor_TLSOptions_From_String(t *testing.T) {
6679
TLSClientKey: string(TLSClientKey),
6780
TLSRootCA: string(TLSRootCA),
6881
}
69-
opts, err := e.TLSOptions(context.Background())
82+
opts, err := e.TLSOptions(ctxWithWorkdir(workdir))
7083
require.NoError(t, err)
7184
require.Len(t, opts, 2)
7285
}
7386

87+
func TestExecutor_TLSOptions_RejectAbsolutePath(t *testing.T) {
88+
e := Executor{
89+
TLSRootCA: "/etc/ssl/certs/ca-certificates.crt",
90+
}
91+
_, err := e.TLSOptions(ctxWithWorkdir(t.TempDir()))
92+
require.Error(t, err)
93+
}
94+
7495
func TestInterpolation_Of_String(t *testing.T) {
7596
e := &Executor{
7697
Method: "",

executors/imap/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Venom IMAP executor implements a few IMAP commands such as FETCH, APPEND, MOVE,
1010
```yaml
1111
auth:
1212
withtls: false
13+
ignore_verify_ssl: false # Optional, default false. Set to true to disable TLS certificate verification (e.g. self-signed certs).
1314
host: yourimaphost
1415
port: 143 # Most probably 993 if using TLS
1516
user: imap@venom.com

executors/plugins/README.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,14 @@ testcases:
7777
- result.body ShouldContainSubstring world
7878
```
7979
80-
Run venom:
80+
Run venom (the `--lib-dir` flag must point to the directory containing the
81+
compiled `.so` plugin; auto-discovery from the testsuite workdir is no longer
82+
supported for security reasons):
8183

8284
```
83-
$ ./venom run test.yml
85+
$ ./venom run --lib-dir=./lib test.yml
8486
```
8587

88+
Plugin names are restricted to the `[A-Za-z0-9_-]` character set.
89+
8690
Feel free to open a Pull Request with your executors.

executors/smtp/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ testcases:
1414
steps:
1515
- type: smtp
1616
withtls: false
17+
ignore_verify_ssl: false # Optional, default false. Set to true to disable TLS certificate verification (e.g. self-signed certs).
1718
host: localhost
1819
port: 25 # 465 if using TLS
1920
user: yourSMTPUsername # Optional, only works with TLS

executors/ssh/README.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,16 @@ In your yaml file, you can use:
1212
- command mandatory
1313
- user optional (default is OS username)
1414
- password optional (mandatory if no privatekey is found)
15-
- privatekey optional (default is $HOME/.ssh/id_rsa)
15+
- privatekey optional (default is $HOME/.ssh/id_rsa).
16+
Relative paths are resolved under the testsuite workdir.
17+
Absolute paths must be located under the user's $HOME.
1618
- sudo optional
1719
- sudopassword optional (default to password)
20+
- insecure_ignore_host_key optional (default false). When false, the server
21+
host key is verified against
22+
$HOME/.ssh/known_hosts. Set to true to
23+
skip verification (insecure).
24+
- timeout optional (default 30). Connection timeout, in seconds.
1825
```
1926
2027
Example

0 commit comments

Comments
 (0)