Allow action generates logs in DetectionOnly mode

[MrWako]

Description

In Deny mode the WAF seems to correctly handle the allow action, and after triggering an allow rule other rules in the same and subsequent phases are ignored and not triggered. In contrast in DetectionOnly mode if the same allow rule is triggered the WAF seems to continue and evaluate subsequent rules. Whilst this doesn't block the request it does generate critical log messages which makes it hard to tune the WAF and be confident that it is correctly configured, before switching to Deny mode.

I guess it might be expected behaviour, although as mentioned it makes switching between DetectionOnly and Deny mode challenging.

Steps to reproduce

import (
	"fmt"
	"log"
	"net/http"
	"net/http/httptest"
	"testing"

	coreruleset "github.com/corazawaf/coraza-coreruleset"
	"github.com/corazawaf/coraza/v3"
	coraza_http "github.com/corazawaf/coraza/v3/http"
	"github.com/corazawaf/coraza/v3/types"
	"github.com/stretchr/testify/assert"
)

func Test_Allow(t *testing.T) {
	// In deny mode we exit the WAF transaction and don't trigger any subsequent rules
	fmt.Println("Testing Deny Mode")
	denyWAF := testPanicWAF(t, true)
	denyH := coraza_http.WrapHandler(denyWAF, http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
		rw.WriteHeader(http.StatusCreated)
	}))
	rw := httptest.NewRecorder()
	r := httptest.NewRequest(http.MethodDelete, "/allowed-path", nil)
	denyH.ServeHTTP(rw, r)
	assert.Equal(t, http.StatusCreated, rw.Code)

	// In detect mode we still log all the rules we break even though we have allowed the request
	fmt.Println("Testing Detect Mode")
	detectWAF := testPanicWAF(t, false)
	detectH := coraza_http.WrapHandler(detectWAF, http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
		rw.WriteHeader(http.StatusCreated)
	}))

	rw = httptest.NewRecorder()
	r = httptest.NewRequest(http.MethodDelete, "/allowed-path", nil)
	detectH.ServeHTTP(rw, r)
	assert.Equal(t, http.StatusCreated, rw.Code)
}

// testPanicWAF is a waf that will panic if it logs anything.
// The WAF is configured so that anything on the route /allowed-path should be allowed through
func testPanicWAF(t *testing.T, deny bool) coraza.WAF {
	errorCallBack := func(err types.MatchedRule) {
		msg := err.Message()
		id := err.Rule().ID()
		file := err.Rule().File()
		serverity := err.Rule().Severity().String()
		uri := err.URI()
		data := err.Data()
		log.Panicf("WAF [%s] %s [%s] ID: %d [%s] URI: %s\n", serverity, msg, data, id, file, uri)
	}

	config := coraza.NewWAFConfig().
		WithErrorCallback(errorCallBack).
		WithRootFS(coreruleset.FS).
		WithDirectives("Include @coraza.conf-recommended").
		WithDirectives("Include @crs-setup.conf.example").
		WithDirectives(`SecRule REQUEST_URI "^/allowed-path" "id:1,phase:1,allow,nolog"`)

	if deny {
		config = config.WithDirectives("SecRuleEngine On")
	} else {
		config = config.WithDirectives("SecRuleEngine DetectionOnly")
	}

	config = config.WithDirectives("Include @owasp_crs/*.conf")

	waf, err := coraza.NewWAF(config)
	assert.NoError(t, err)
	return waf
}

Expected result

No logs generated for either Deny or Detect.

Actual result

output from the test

Testing Deny Mode
Testing Detect Mode
2024/04/25 21:20:06 WAF [critical] Method is not allowed by policy [DELETE] ID: 911100 [@owasp_crs/REQUEST-911-METHOD-ENFORCEMENT.conf] URI: /allowed-path
--- FAIL: Test_Allow (0.14s)
panic: WAF [critical] Method is not allowed by policy [DELETE] ID: 911100 [@owasp_crs/REQUEST-911-METHOD-ENFORCEMENT.conf] URI: /allowed-path
 [recovered]
	panic: WAF [critical] Method is not allowed by policy [DELETE] ID: 911100 [@owasp_crs/REQUEST-911-METHOD-ENFORCEMENT.conf] URI: /allowed-path