Repository: corazawaf/coraza
Pull request #449 (2022-09-30 09:55:36)

feat: adds support for stdout.
This PR adds support for stdout as output for debug log. Most of the references to stdout in modsec world points to `/dev/stdout` as the target although it isn't idiomatic to Windows.

> Thank you for contributing to Coraza WAF, your effort is greatly appreciated
> Before submitting check if what you want to add to `coraza` list meets [quality standards](https://github.com/corazawaf/coraza/blob/master/CONTRIBUTING.md#quality-standards) before sending pull request. Thanks!

**Make sure that you've checked the boxes below before you submit PR:**

- [ ] My code includes positive and negative tests.
- [ ] I have an appropriate description with correct grammar.
- [ ] I have read [Contribution guidelines](https://github.com/corazawaf/coraza/blob/master/CONTRIBUTING.md#contribution-guidelines), [maintainers note](https://github.com/corazawaf/coraza/blob/master/CONTRIBUTING.md#maintainers) and [Quality standard](https://github.com/github.com/corazawaf/coraza/v3sso/coraza-waf/blob/master/CONTRIBUTING.md#quality-standards).
- [ ] My code is properly linted and passes pre-commit tests.

Thanks for your contribution :heart:

jcchavezs: @fzipri actually this is debug log and most of the usages I saw (did a
quick research in GitHub/Google) was pointing to /dev/stdout. More so this
started as a need for us to display logs in stdout so I would hold the
addition of stderr until someone request for it.

On Fri, 30 Sept 2022, 14:16 Felipe Zipitría, ***@***.***>
wrote:

> ***@***.**** commented on this pull request.
> ------------------------------
>
> In internal/corazawaf/waf.go
> <https://github.com/corazawaf/coraza/pull/449#discussion_r984528688>:
>
> > @@ -420,9 +420,15 @@ func (w *WAF) SetDebugLogPath(path string) error {
>  		w.Logger.SetOutput(io.Discard)
>  		return nil
>  	}
> +
> +	if path == "/dev/stdout" {
>
> What about using stderr? Should this deserve also a case for it?
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/corazawaf/coraza/pull/449#discussion_r984528688>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAXOYAVLI5NLPZRBJHONMUDWA3KZRANCNFSM6AAAAAAQZTU3XY>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>

jcchavezs: I added support for `stderr` as per @fzipi and @anuraaga suggestion.

Patch:
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
new file mode 100644
index 0000000..ccbb900
--- /dev/null
+++ b/.github/CODEOWNERS
@@ -0,0 +1,3 @@
+# https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners
+
+* @corazawaf/core-developers
diff --git a/examples/http-server/main.go b/examples/http-server/main.go
index b5727a3..e80311d 100644
--- a/examples/http-server/main.go
+++ b/examples/http-server/main.go
@@ -24,15 +24,18 @@ func main() {
 }
 
 func createWAF() coraza.WAF {
-	waf, err := coraza.NewWAF(coraza.NewWAFConfig().
-		WithDirectives(`
-		# This is a comment
-		SecDebugLogLevel 9
-		SecRequestBodyAccess On
-		SecRule ARGS:id "@eq 0" "id:1, phase:1,deny, status:403,msg:'Invalid id',log,auditlog"
-		SecRule REQUEST_BODY "somecontent" "id:100, phase:2,deny, status:403,msg:'Invalid request body',log,auditlog"
-		SecRule RESPONSE_BODY "somecontent" "id:200, phase:4,deny, status:403,msg:'Invalid response body',log,auditlog"
-	`))
+	waf, err := coraza.NewWAF(
+		coraza.NewWAFConfig().
+			WithDirectives(`
+				# This is a comment
+				SecDebugLogLevel 5
+				SecRequestBodyAccess On
+				SecDebugLog /dev/stdout
+				SecRule ARGS:id "@eq 0" "id:1, phase:1,deny, status:403,msg:'Invalid id',log,auditlog"
+				SecRule REQUEST_BODY "somecontent" "id:100, phase:2,deny, status:403,msg:'Invalid request body',log,auditlog"
+				SecRule RESPONSE_BODY "somecontent" "id:200, phase:4,deny, status:403,msg:'Invalid response body',log,auditlog"
+			`),
+	)
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/internal/corazawaf/logger.go b/internal/corazawaf/logger.go
index 84546da..aa2341f 100644
--- a/internal/corazawaf/logger.go
+++ b/internal/corazawaf/logger.go
@@ -17,6 +17,8 @@ type stdDebugLogger struct {
 	Level  loggers.LogLevel
 }
 
+var _ loggers.DebugLogger = (*stdDebugLogger)(nil)
+
 func (l *stdDebugLogger) formatLog(level loggers.LogLevel, message string, args ...interface{}) {
 	if l.Level >= level {
 		l.logger.Printf("[%s] %s", level.String(), fmt.Sprintf(message, args...))
diff --git a/internal/corazawaf/waf.go b/internal/corazawaf/waf.go
index 23f6c48..e8ffcdc 100644
--- a/internal/corazawaf/waf.go
+++ b/internal/corazawaf/waf.go
@@ -409,7 +409,7 @@ func (w *WAF) NewTransaction(ctx context.Context) *Transaction {
 	tx.Variables.HighestSeverity.Set("0")
 	tx.Variables.UniqueID.Set(tx.ID)
 
-	w.Logger.Debug("new transaction created with id %q", tx.ID)
+	w.Logger.Debug("New transaction created with id %q", tx.ID)
 
 	return tx
 }
@@ -422,9 +422,20 @@ func (w *WAF) SetDebugLogPath(path string) error {
 		w.Logger.SetOutput(io.Discard)
 		return nil
 	}
+
+	if path == "/dev/stdout" {
+		w.Logger.SetOutput(os.Stdout)
+		return nil
+	}
+
+	if path == "/dev/stderr" {
+		w.Logger.SetOutput(os.Stderr)
+		return nil
+	}
+
 	f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666)
 	if err != nil {
-		w.Logger.Error("error opening file: %s", err.Error())
+		w.Logger.Error("failed to open the file: %s", err.Error())
 	}
 	w.Logger.SetOutput(f)
 	return nil


Test patch:
diff --git a/internal/corazawaf/waf_test.go b/internal/corazawaf/waf_test.go
index 36fb4b2..bd6dc71 100644
--- a/internal/corazawaf/waf_test.go
+++ b/internal/corazawaf/waf_test.go
@@ -5,15 +5,11 @@ package corazawaf
 
 import (
 	"context"
+	"io"
+	"os"
 	"testing"
 )
 
-var waf *WAF
-
-func TestWAFInitialize(t *testing.T) {
-	waf = NewWAF()
-}
-
 func TestNewTransaction(t *testing.T) {
 	waf := NewWAF()
 	waf.RequestBodyAccess = true
@@ -30,3 +26,36 @@ func TestNewTransaction(t *testing.T) {
 		t.Error("Request body limit not set")
 	}
 }
+
+func TestSetDebugLogPath(t *testing.T) {
+	waf := NewWAF()
+
+	testCases := []struct {
+		path   string
+		writer io.Writer
+	}{
+		{
+			path:   "/dev/stdout",
+			writer: os.Stdout,
+		},
+		{
+			path:   "/dev/stderr",
+			writer: os.Stderr,
+		},
+	}
+
+	for _, tCase := range testCases {
+		t.Run(tCase.path, func(t *testing.T) {
+			err := waf.SetDebugLogPath(tCase.path)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			l := waf.Logger.(*stdDebugLogger)
+			if want, have := tCase.writer, l.logger.Writer(); want != have {
+				t.Error("unexpected logger writer")
+			}
+			_ = waf.SetDebugLogPath("")
+		})
+	}
+}