(incubator-devlake) branch main updated: feat: forbid starting tx via the dalgorm.Exec (#6685)

[ab...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

abeizn pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-devlake.git


The following commit(s) were added to refs/heads/main by this push:
     new 046c0baa2 feat: forbid starting tx via the dalgorm.Exec (#6685)
046c0baa2 is described below

commit 046c0baa235f555c06ecda30e7f468e9f516cb7b
Author: Klesh Wong <zh...@merico.dev>
AuthorDate: Thu Dec 21 17:24:05 2023 +0800

    feat: forbid starting tx via the dalgorm.Exec (#6685)
    
    * feat: forbid starting tx via the dalgorm.Exec
    
    * chore: add positive cases for vlidateQuery
---
 backend/impls/dalgorm/dalgorm.go      | 14 ++++++++++
 backend/impls/dalgorm/dalgorm_test.go | 50 +++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/backend/impls/dalgorm/dalgorm.go b/backend/impls/dalgorm/dalgorm.go
index c5518a889..a3de41123 100644
--- a/backend/impls/dalgorm/dalgorm.go
+++ b/backend/impls/dalgorm/dalgorm.go
@@ -21,6 +21,7 @@ import (
 	"database/sql"
 	"fmt"
 	"reflect"
+	"regexp"
 	"strings"
 
 	"github.com/apache/incubator-devlake/core/dal"
@@ -138,9 +139,22 @@ var _ dal.Dal = (*Dalgorm)(nil)
 
 // Exec executes raw sql query
 func (d *Dalgorm) Exec(query string, params ...interface{}) errors.Error {
+	err := validateQuery(query)
+	if err != nil {
+		return err
+	}
 	return d.convertGormError(d.db.Exec(query, transformParams(params)...).Error)
 }
 
+var txPattern = regexp.MustCompile(`(?i)^[\s;]*(begin|(start\s+transaction))[\s;]*$`)
+
+func validateQuery(query string) errors.Error {
+	if txPattern.MatchString(query) { // regexp.MatchString is thread-safe
+		return errors.Default.New("illegal invocation, use the `Begin()` method instead")
+	}
+	return nil
+}
+
 func (d *Dalgorm) unwrapDynamic(entityPtr *interface{}, clausesPtr *[]dal.Clause) {
 	if dynamic, ok := (*entityPtr).(models.DynamicTabler); ok {
 		if clausesPtr != nil {
diff --git a/backend/impls/dalgorm/dalgorm_test.go b/backend/impls/dalgorm/dalgorm_test.go
new file mode 100644
index 000000000..b6081b988
--- /dev/null
+++ b/backend/impls/dalgorm/dalgorm_test.go
@@ -0,0 +1,50 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package dalgorm
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_validateQuery(t *testing.T) {
+	for _, target := range []string{
+		"begin",
+		" begin",
+		"begin ",
+		" begin ",
+		"begin;",
+		"begin ;",
+		"begin ; ;",
+		"BEGIN ; ;",
+		"start transaction",
+		"start   transaction",
+		"START   TRANSACTION",
+		"start\t\n   transaction",
+		" ;; start transaction",
+	} {
+		assert.EqualError(t, validateQuery(target), "illegal invocation, use the `Begin()` method instead", "failed text: `%s`", target)
+	}
+	for _, target := range []string{
+		"select 1",
+		"update a set b = c",
+	} {
+		assert.Nil(t, validateQuery(target), "failed text: `%s`", target)
+	}
+}