From e0b254286806d4517ea8ce69b6cb25a044b5a1ef Mon Sep 17 00:00:00 2001 From: Dwi Siswanto Date: Wed, 28 Aug 2024 19:27:45 +0700 Subject: [PATCH] feat: conditionally panic-recover (#5553) * feat: conditionally panic-recover As discussed with @Mzack9999, we should avoid overusing panic-recover. We need to review the RCA first to determine whether this is an exceptional situation or if it's a higher-level function meant to recover from a panic. This approach will help us establish a robust error-handling strategy. The implementation of panic-recover should be conditional and NOT applied when running in a CI environment AND IS temporary. Once we've caught all errors and made the necessary corrections, we can remove the deferred recover function. Signed-off-by: Dwi Siswanto * chore(deps): bump `go-ci` to v1.0.2 Signed-off-by: Dwi Siswanto * chore(make): add `-race` to `GOFLAGS` in `test` Signed-off-by: Dwi Siswanto --------- Signed-off-by: Dwi Siswanto --- Makefile | 1 + go.mod | 1 + go.sum | 4 ++++ pkg/js/compiler/compiler.go | 7 +++++++ pkg/js/compiler/pool.go | 8 ++++++++ pkg/protocols/headless/engine/page_actions.go | 6 ++++++ pkg/tmplexec/flow/flow_executor.go | 7 +++++++ 7 files changed, 34 insertions(+) diff --git a/Makefile b/Makefile index 3fa6a033ef..8614ef82c9 100644 --- a/Makefile +++ b/Makefile @@ -65,6 +65,7 @@ docs: git reset --hard # line 59 +test: GOFLAGS = -race -v test: $(GOTEST) $(GOFLAGS) ./... diff --git a/go.mod b/go.mod index a0cd13984a..858a21bca3 100644 --- a/go.mod +++ b/go.mod @@ -177,6 +177,7 @@ require ( github.com/jmespath/go-jmespath v0.4.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/kataras/jwt v0.1.10 // indirect + github.com/kitabisa/go-ci v1.0.2 // indirect github.com/klauspost/compress v1.17.8 // indirect github.com/klauspost/pgzip v1.2.6 // indirect github.com/kylelemons/godebug v1.1.0 // indirect diff --git a/go.sum b/go.sum index 310bd42ef8..66c91ae901 100644 --- a/go.sum +++ b/go.sum @@ -625,6 +625,10 @@ github.com/kevinburke/ssh_config v1.2.0/go.mod h1:CT57kijsi8u/K/BOFA39wgDQJ9CxiF github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= +github.com/kitabisa/go-ci v1.0.1 h1://FHQzlDqYW+3qr0judsOE9X2ZrdRlRc66sCaVrLFGc= +github.com/kitabisa/go-ci v1.0.1/go.mod h1:4MWu+kf/+tvd0vLWSJA689Kn+hrYkZiymmZYT5BGT4g= +github.com/kitabisa/go-ci v1.0.2 h1:rqHf8KEbQOxVb998TbqGRo70Z7ol44io7/jLYJUvKp8= +github.com/kitabisa/go-ci v1.0.2/go.mod h1:e3wBSzaJbcifXrr/Gw2ZBLn44MmeqP5WySwXyHlCK/U= github.com/klauspost/compress v1.4.1/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A= github.com/klauspost/compress v1.11.4/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs= github.com/klauspost/compress v1.17.8 h1:YcnTYrq7MikUT7k0Yb5eceMmALQPYBW/Xltxn0NAMnU= diff --git a/pkg/js/compiler/compiler.go b/pkg/js/compiler/compiler.go index c2dc15d2cb..b13e7f9ecb 100644 --- a/pkg/js/compiler/compiler.go +++ b/pkg/js/compiler/compiler.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/dop251/goja" + "github.com/kitabisa/go-ci" "github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/generators" "github.com/projectdiscovery/nuclei/v3/pkg/types" @@ -120,11 +121,17 @@ func (c *Compiler) ExecuteWithOptions(program *goja.Program, args *ExecuteArgs, defer cancel() // execute the script results, err := contextutil.ExecFuncWithTwoReturns(ctx, func() (val goja.Value, err error) { + // TODO(dwisiswant0): remove this once we get the RCA. defer func() { + if ci.IsCI() { + return + } + if r := recover(); r != nil { err = fmt.Errorf("panic: %v", r) } }() + return ExecuteProgram(program, args, opts) }) if err != nil { diff --git a/pkg/js/compiler/pool.go b/pkg/js/compiler/pool.go index 3407d97369..31e2e5378f 100644 --- a/pkg/js/compiler/pool.go +++ b/pkg/js/compiler/pool.go @@ -11,6 +11,7 @@ import ( "github.com/dop251/goja" "github.com/dop251/goja_nodejs/console" "github.com/dop251/goja_nodejs/require" + "github.com/kitabisa/go-ci" "github.com/projectdiscovery/gologger" _ "github.com/projectdiscovery/nuclei/v3/pkg/js/generated/go/libbytes" _ "github.com/projectdiscovery/nuclei/v3/pkg/js/generated/go/libfs" @@ -84,11 +85,18 @@ func executeWithRuntime(runtime *goja.Runtime, p *goja.Program, args *ExecuteArg opts.Cleanup(runtime) } }() + + // TODO(dwisiswant0): remove this once we get the RCA. defer func() { + if ci.IsCI() { + return + } + if r := recover(); r != nil { err = fmt.Errorf("panic: %s", r) } }() + // set template ctx _ = runtime.Set("template", args.TemplateCtx) // set args diff --git a/pkg/protocols/headless/engine/page_actions.go b/pkg/protocols/headless/engine/page_actions.go index 57cc371093..576b6134fa 100644 --- a/pkg/protocols/headless/engine/page_actions.go +++ b/pkg/protocols/headless/engine/page_actions.go @@ -14,6 +14,7 @@ import ( "github.com/go-rod/rod/lib/input" "github.com/go-rod/rod/lib/proto" "github.com/go-rod/rod/lib/utils" + "github.com/kitabisa/go-ci" "github.com/pkg/errors" "github.com/projectdiscovery/gologger" "github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/contextargs" @@ -54,7 +55,12 @@ func (p *Page) ExecuteActions(input *contextargs.Context, actions []*Action, var waitFuncs := make([]func() error, 0) // avoid any future panics caused due to go-rod library + // TODO(dwisiswant0): remove this once we get the RCA. defer func() { + if ci.IsCI() { + return + } + if r := recover(); r != nil { err = errorutil.New("panic on headless action: %v", r) } diff --git a/pkg/tmplexec/flow/flow_executor.go b/pkg/tmplexec/flow/flow_executor.go index 6a1813efd9..6e71cf8407 100644 --- a/pkg/tmplexec/flow/flow_executor.go +++ b/pkg/tmplexec/flow/flow_executor.go @@ -13,6 +13,7 @@ import ( "github.com/projectdiscovery/nuclei/v3/pkg/scan" templateTypes "github.com/projectdiscovery/nuclei/v3/pkg/templates/types" + "github.com/kitabisa/go-ci" "github.com/projectdiscovery/nuclei/v3/pkg/types" errorutil "github.com/projectdiscovery/utils/errors" fileutil "github.com/projectdiscovery/utils/file" @@ -201,7 +202,13 @@ func (f *FlowExecutor) ExecuteWithResults(ctx *scan.ScanContext) error { } }() + + // TODO(dwisiswant0): remove this once we get the RCA. defer func() { + if ci.IsCI() { + return + } + if r := recover(); r != nil { f.ctx.LogError(fmt.Errorf("panic occurred while executing flow: %v", r)) }