Skip to content

Commit

Permalink
feat(api): strict variable usage for lua, check result is bool (#5757)
Browse files Browse the repository at this point in the history
  • Loading branch information
@richardlt
richardlt authored Mar 11, 2021
1 parent f227609 commit a239e4a
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 22 deletions.
15 changes: 10 additions & 5 deletions engine/api/purge/purge_run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,20 +135,26 @@ func applyRetentionPolicyOnRun(ctx context.Context, db *gorp.DbMap, wf sdk.Workf
}
return false, nil
}
luacheck, err := luascript.NewCheck()

luaCheck, err := luascript.NewCheck()
if err != nil {
return true, sdk.WithStack(err)
}

if err := purgeComputeVariables(ctx, luacheck, run, branchesMap, app, vcsClient); err != nil {
if err := purgeComputeVariables(ctx, luaCheck, run, branchesMap, app, vcsClient); err != nil {
return true, err
}

if err := luacheck.Perform(wf.RetentionPolicy); err != nil {
// Enabling strict checks on variables to prevent errors on rule definition
if err := luaCheck.EnableStrict(); err != nil {
return true, sdk.WithStack(err)
}

if err := luaCheck.Perform(wf.RetentionPolicy); err != nil {
return true, sdk.NewErrorFrom(sdk.ErrWrongRequest, "unable to apply retention policy on workflow %s/%s: %v", wf.ProjectKey, wf.Name, err)
}

if luacheck.Result {
if luaCheck.Result {
return true, nil
}
if !opts.DryRun {
Expand Down Expand Up @@ -203,7 +209,6 @@ func purgeComputeVariables(ctx context.Context, luaCheck *luascript.Check, run s
vars[RunChangeAbandoned] = strconv.FormatBool(ch.Closed)
varsFloats[RunChangeDayBefore] = math.Floor(time.Now().Sub(ch.Updated).Hours())
}

}

// If we have a branch in payload, check if it exists on repository branches list
Expand Down
48 changes: 48 additions & 0 deletions engine/api/purge/purge_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,51 @@ func Test_applyRetentionPolicyOnRun(t *testing.T) {
require.NoError(t, err)
require.True(t, keep)
}

func Test_applyRetentionPolicyOnRunWithError(t *testing.T) {
db, _ := test.SetupPG(t, bootstrap.InitiliazeDB)

// check empty rule
keep, err := applyRetentionPolicyOnRun(context.TODO(), db.DbMap, sdk.Workflow{
RetentionPolicy: "",
}, sdk.WorkflowRunSummary{}, nil, sdk.Application{}, nil, MarkAsDeleteOptions{DryRun: true})
require.Error(t, err)
require.True(t, keep)

// check no return
keep, err = applyRetentionPolicyOnRun(context.TODO(), db.DbMap, sdk.Workflow{
RetentionPolicy: "unknown == 'true'",
}, sdk.WorkflowRunSummary{}, nil, sdk.Application{}, nil, MarkAsDeleteOptions{DryRun: true})
require.Error(t, err)
require.True(t, keep)

// check unknown variable
keep, err = applyRetentionPolicyOnRun(context.TODO(), db.DbMap, sdk.Workflow{
RetentionPolicy: "return unknown == 'true'",
}, sdk.WorkflowRunSummary{}, nil, sdk.Application{}, nil, MarkAsDeleteOptions{DryRun: true})
require.Error(t, err)
require.True(t, keep)

// check return nil
keep, err = applyRetentionPolicyOnRun(context.TODO(), db.DbMap, sdk.Workflow{
RetentionPolicy: "return nil",
}, sdk.WorkflowRunSummary{}, nil, sdk.Application{}, nil, MarkAsDeleteOptions{DryRun: true})
require.Error(t, err)
require.True(t, keep)

keep, err = applyRetentionPolicyOnRun(context.TODO(), db.DbMap, sdk.Workflow{
RetentionPolicy: "return run_status == 'Success'",
}, sdk.WorkflowRunSummary{
Status: sdk.StatusSuccess,
}, nil, sdk.Application{}, nil, MarkAsDeleteOptions{DryRun: true})
require.NoError(t, err)
require.True(t, keep)

keep, err = applyRetentionPolicyOnRun(context.TODO(), db.DbMap, sdk.Workflow{
RetentionPolicy: "return run_status ~= 'Success'",
}, sdk.WorkflowRunSummary{
Status: sdk.StatusSuccess,
}, nil, sdk.Application{}, nil, MarkAsDeleteOptions{DryRun: true})
require.NoError(t, err)
require.False(t, keep)
}
27 changes: 27 additions & 0 deletions sdk/luascript/luascript.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package luascript

import (
"context"
"fmt"
"strings"

"github.com/yuin/gluare"
Expand Down Expand Up @@ -85,6 +86,28 @@ func (c *Check) SetFloatVariables(vars map[string]float64) {
}
}

func (c *Check) EnableStrict() error {
// This code will override __index lua func that is called when reading a variable.
// If the variable is not define we want to return an error.
code := `
local mt = getmetatable(_G)
if mt == nil then
mt = {}
setmetatable(_G, mt)
end
mt.__index = function (t, n)
if n ~= "C" then
error("variable '"..n.."' is not declared", 2)
end
return rawget(t, n)
end
`
if err := c.state.DoString(code); err != nil {
return err
}
return nil
}

//Perform the lua script
func (c *Check) Perform(script string) error {
var ok bool
Expand All @@ -95,6 +118,10 @@ func (c *Check) Perform(script string) error {
}

lv := c.state.Get(-1) // get the value at the top of the stack
if lv.Type() != lua.LTBool {
c.IsError = true
return fmt.Errorf("invalid result of type %s expected boolean", lv.Type().String())
}
if lua.LVAsBool(lv) { // lv is neither nil nor false
ok = true
}
Expand Down
79 changes: 62 additions & 17 deletions sdk/luascript/luascript_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,91 +3,136 @@ package luascript
import (
"testing"

"github.com/ovh/cds/engine/api/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestLuaCheck(t *testing.T) {
l, err := NewCheck()
test.NoError(t, err)
require.NoError(t, err)
l.SetVariables(map[string]string{
"cds.application": "mon-appli",
})
test.NoError(t, l.Perform("return cds_application == \"mon-appli\""))
require.NoError(t, l.Perform("return cds_application == \"mon-appli\""))
assert.False(t, l.IsError)
assert.True(t, l.Result)
}

func TestLuaPerformErrorNoBoolReturn(t *testing.T) {
l1, err := NewCheck()
require.NoError(t, err)
require.Error(t, l1.Perform(""))
require.True(t, l1.IsError)
require.False(t, l1.Result)

l2, err := NewCheck()
require.NoError(t, err)
require.Error(t, l2.Perform("return nil"))
require.True(t, l2.IsError)
require.False(t, l2.Result)
}

func TestLuaCheckStrings(t *testing.T) {
l, err := NewCheck()
test.NoError(t, err)
require.NoError(t, err)
l.SetVariables(map[string]string{
"cds.application": "mon-appli",
})
test.NoError(t, l.Perform("return string.match(\"abcdefg\", \"b..\") == \"bcd\""))
require.NoError(t, l.Perform("return string.match(\"abcdefg\", \"b..\") == \"bcd\""))
assert.False(t, l.IsError)
assert.True(t, l.Result)
}

func TestLuaCheckStringsFind(t *testing.T) {
l, err := NewCheck()
test.NoError(t, err)
require.NoError(t, err)
l.SetVariables(map[string]string{
"git_branch": "release/foo",
})
test.NoError(t, l.Perform(`return git_branch:find("^release/") ~= nil`))
require.NoError(t, l.Perform(`return git_branch:find("^release/") ~= nil`))
assert.False(t, l.IsError)
assert.True(t, l.Result)
}

func TestLuaCheckWeekOfDay(t *testing.T) {
l, err := NewCheck()
test.NoError(t, err)
require.NoError(t, err)
l.SetVariables(map[string]string{
"cds.application": "mon-appli",
})
test.NoError(t, l.Perform(`return os.date("%w") < "8"`))
require.NoError(t, l.Perform(`return os.date("%w") < "8"`))
assert.False(t, l.IsError)
assert.True(t, l.Result)
}

func TestLuaNilVariables(t *testing.T) {
script := `local re = require("re")
return
(git_branch ~= nil and re.match(git_branch,"integration") == "integration") or
(git_repository ~= nil and re.match(git_repository,"integration") == "integration") or
return
(git_branch ~= nil and re.match(git_branch,"integration") == "integration") or
(git_repository ~= nil and re.match(git_repository,"integration") == "integration") or
(git_pr_title ~= nil and re.match(git_pr_title,"integration") == "integration")`

l, err := NewCheck()
test.NoError(t, err)
require.NoError(t, err)
l.SetVariables(map[string]string{
"git_repository": "PROJECT/integration",
})
test.NoError(t, l.Perform(script))
require.NoError(t, l.Perform(script))
assert.False(t, l.IsError)
assert.True(t, l.Result)
}

func TestLuaCheckRegularExpression(t *testing.T) {
l, err := NewCheck()
test.NoError(t, err)
require.NoError(t, err)
l.SetVariables(map[string]string{
"cds.application": "mon-appli",
})
test.NoError(t, l.Perform(`
require.NoError(t, l.Perform(`
local re = require("re")
return re.match("abcdefg", "abc.*") == "abcdefg"
`))
assert.False(t, l.IsError)
assert.True(t, l.Result)

test.NoError(t, l.Perform(`
require.NoError(t, l.Perform(`
local re = require("re")
return re.match("abcdefg", "zzz.*") == ""
`))
assert.False(t, l.IsError)
assert.False(t, l.Result)
}

func Test_luaPerformStrictCheckOnVariable(t *testing.T) {
luaCheck, err := NewCheck()
require.NoError(t, err)

luaCheck.SetVariables(map[string]string{
"defined_string": "value",
})
luaCheck.SetFloatVariables(map[string]float64{
"defined_number": 123,
})

require.NoError(t, luaCheck.EnableStrict())

require.Error(t, luaCheck.Perform("return undefined_string == 'value'"))
require.False(t, luaCheck.Result)
require.Error(t, luaCheck.Perform("return undefined_string ~= 'value'"))
require.False(t, luaCheck.Result)
require.NoError(t, luaCheck.Perform("return defined_string == 'value'"))
require.True(t, luaCheck.Result)
require.NoError(t, luaCheck.Perform("return defined_string ~= 'value'"))
require.False(t, luaCheck.Result)

require.Error(t, luaCheck.Perform("return undefined_number < 1000"))
require.False(t, luaCheck.Result)
require.Error(t, luaCheck.Perform("return undefined_number < 100"))
require.False(t, luaCheck.Result)
require.NoError(t, luaCheck.Perform("return defined_number < 1000"))
require.True(t, luaCheck.Result)
require.NoError(t, luaCheck.Perform("return defined_number < 100"))
require.False(t, luaCheck.Result)
}

0 comments on commit a239e4a

Please sign in to comment.