Skip to content

Commit

Permalink
[CRE-44] Add restricted configuration; validate transformer values (#…
Browse files Browse the repository at this point in the history
…15961)

* [CRE-44] Restricted config

* [CRE-44] Validate values passed into Compute transformer

* [fix] Remove debug logging in safeurl

* Update common

* <= 0
  • Loading branch information
cedric-cordenier authored Jan 21, 2025
1 parent 2ce2523 commit 6facc69
Show file tree
Hide file tree
Showing 17 changed files with 302 additions and 44 deletions.
33 changes: 26 additions & 7 deletions core/capabilities/compute/compute.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,12 +360,33 @@ func (c *Compute) createFetcher() func(ctx context.Context, req *wasmpb.FetchReq
}

const (
defaultNumWorkers = 3
defaultNumWorkers = 3
defaultMaxMemoryMBs = 128
defaultMaxTickInterval = 100 * time.Millisecond
defaultMaxTimeout = 10 * time.Second
)

type Config struct {
webapi.ServiceConfig
NumWorkers int
NumWorkers int
MaxMemoryMBs uint64
MaxTimeout time.Duration
MaxTickInterval time.Duration
}

func (c *Config) ApplyDefaults() {
if c.NumWorkers == 0 {
c.NumWorkers = defaultNumWorkers
}
if c.MaxMemoryMBs == 0 {
c.MaxMemoryMBs = defaultMaxMemoryMBs
}
if c.MaxTimeout == 0 {
c.MaxTimeout = defaultMaxTimeout
}
if c.MaxTickInterval == 0 {
c.MaxTickInterval = defaultMaxTickInterval
}
}

func NewAction(
Expand All @@ -376,9 +397,7 @@ func NewAction(
idGenerator func() string,
opts ...func(*Compute),
) (*Compute, error) {
if config.NumWorkers == 0 {
config.NumWorkers = defaultNumWorkers
}
config.ApplyDefaults()
metricsLabeler, err := newComputeMetricsLabeler(metrics.NewLabeler().With("capability", CapabilityIDCompute))
if err != nil {
return nil, fmt.Errorf("failed to create compute metrics labeler: %w", err)
Expand All @@ -393,11 +412,11 @@ func NewAction(
metrics: metricsLabeler,
registry: registry,
modules: newModuleCache(clockwork.NewRealClock(), 1*time.Minute, 10*time.Minute, 3),
transformer: NewTransformer(lggr, labeler),
transformer: NewTransformer(lggr, labeler, config),
outgoingConnectorHandler: handler,
idGenerator: idGenerator,
queue: make(chan request),
numWorkers: defaultNumWorkers,
numWorkers: config.NumWorkers,
}
)

Expand Down
21 changes: 19 additions & 2 deletions core/capabilities/compute/transformer.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type ParsedConfig struct {
type transformer struct {
logger logger.Logger
emitter custmsg.MessageEmitter
config Config
}

func shallowCopy(m *values.Map) *values.Map {
Expand Down Expand Up @@ -60,11 +61,15 @@ func (t *transformer) Transform(req capabilities.CapabilityRequest, opts ...func
return capabilities.CapabilityRequest{}, nil, NewInvalidRequestError(err)
}

maxMemoryMBs, err := popOptionalValue[int64](copiedReq.Config, maxMemoryMBsKey)
maxMemoryMBs, err := popOptionalValue[uint64](copiedReq.Config, maxMemoryMBsKey)
if err != nil {
return capabilities.CapabilityRequest{}, nil, NewInvalidRequestError(err)
}

if maxMemoryMBs <= 0 || maxMemoryMBs > t.config.MaxMemoryMBs {
maxMemoryMBs = t.config.MaxMemoryMBs
}

mc := &host.ModuleConfig{
MaxMemoryMBs: maxMemoryMBs,
Logger: t.logger,
Expand All @@ -82,9 +87,16 @@ func (t *transformer) Transform(req capabilities.CapabilityRequest, opts ...func
if err != nil {
return capabilities.CapabilityRequest{}, nil, NewInvalidRequestError(err)
}
if td <= 0 || td > t.config.MaxTimeout {
td = t.config.MaxTimeout
}
mc.Timeout = &td
}

if mc.Timeout == nil {
mc.Timeout = &t.config.MaxTimeout
}

tickInterval, err := popOptionalValue[string](copiedReq.Config, tickIntervalKey)
if err != nil {
return capabilities.CapabilityRequest{}, nil, NewInvalidRequestError(err)
Expand All @@ -99,6 +111,10 @@ func (t *transformer) Transform(req capabilities.CapabilityRequest, opts ...func
mc.TickInterval = ti
}

if mc.TickInterval <= 0 || mc.TickInterval > t.config.MaxTickInterval {
mc.TickInterval = t.config.MaxTickInterval
}

pc := &ParsedConfig{
Binary: binary,
Config: config,
Expand All @@ -112,10 +128,11 @@ func (t *transformer) Transform(req capabilities.CapabilityRequest, opts ...func
return copiedReq, pc, nil
}

func NewTransformer(lggr logger.Logger, emitter custmsg.MessageEmitter) *transformer {
func NewTransformer(lggr logger.Logger, emitter custmsg.MessageEmitter, config Config) *transformer {
return &transformer{
logger: lggr,
emitter: emitter,
config: config,
}
}

Expand Down
126 changes: 120 additions & 6 deletions core/capabilities/compute/transformer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,12 @@ func Test_transformer(t *testing.T) {
},
}

tf := NewTransformer(lgger, emitter)
config := Config{
MaxMemoryMBs: 2048,
MaxTimeout: 20 * time.Second,
MaxTickInterval: 10 * time.Second,
}
tf := NewTransformer(lgger, emitter, config)
_, gotConfig, err := tf.Transform(giveReq)

require.NoError(t, err)
Expand All @@ -130,16 +135,22 @@ func Test_transformer(t *testing.T) {
}
require.NoError(t, err)

timeout := defaultMaxTimeout
wantConfig := &ParsedConfig{
Binary: []byte{0x01, 0x02, 0x03},
Config: []byte{0x04, 0x05, 0x06},
ModuleConfig: &host.ModuleConfig{
Logger: lgger,
Labeler: emitter,
Logger: lgger,
Labeler: emitter,
TickInterval: defaultMaxTickInterval,
Timeout: &timeout,
MaxMemoryMBs: defaultMaxMemoryMBs,
},
}

tf := NewTransformer(lgger, emitter)
config := Config{}
config.ApplyDefaults()
tf := NewTransformer(lgger, emitter, config)
_, gotConfig, err := tf.Transform(giveReq)

require.NoError(t, err)
Expand All @@ -157,7 +168,9 @@ func Test_transformer(t *testing.T) {
}
require.NoError(t, err)

tf := NewTransformer(lgger, emitter)
config := Config{}
config.ApplyDefaults()
tf := NewTransformer(lgger, emitter, config)
_, _, err = tf.Transform(giveReq)

require.Error(t, err)
Expand All @@ -175,10 +188,111 @@ func Test_transformer(t *testing.T) {
}
require.NoError(t, err)

tf := NewTransformer(lgger, emitter)
config := Config{}
config.ApplyDefaults()
tf := NewTransformer(lgger, emitter, config)
_, _, err = tf.Transform(giveReq)

require.Error(t, err)
require.ErrorContains(t, err, "invalid request")
})

t.Run("invalid tickInterval, applies default", func(t *testing.T) {
giveMap, err := values.NewMap(map[string]any{
"tickInterval": "-50ms",
"binary": []byte{0x01, 0x02, 0x03},
"config": []byte{0x04, 0x05, 0x06},
})
giveReq := capabilities.CapabilityRequest{
Config: giveMap,
}
require.NoError(t, err)

config := Config{}
config.ApplyDefaults()
tf := NewTransformer(lgger, emitter, config)
_, pc, err := tf.Transform(giveReq)

require.NoError(t, err)
assert.Equal(t, defaultMaxTickInterval, pc.ModuleConfig.TickInterval)
})

t.Run("invalid timeout, applies default", func(t *testing.T) {
giveMap, err := values.NewMap(map[string]any{
"timeout": "-50ms",
"binary": []byte{0x01, 0x02, 0x03},
"config": []byte{0x04, 0x05, 0x06},
})
giveReq := capabilities.CapabilityRequest{
Config: giveMap,
}
require.NoError(t, err)

config := Config{}
config.ApplyDefaults()
tf := NewTransformer(lgger, emitter, config)
_, pc, err := tf.Transform(giveReq)

require.NoError(t, err)
assert.Equal(t, defaultMaxTimeout, *pc.ModuleConfig.Timeout)
})

t.Run("timeout too high, applies default", func(t *testing.T) {
giveMap, err := values.NewMap(map[string]any{
"timeout": "1h",
"binary": []byte{0x01, 0x02, 0x03},
"config": []byte{0x04, 0x05, 0x06},
})
giveReq := capabilities.CapabilityRequest{
Config: giveMap,
}
require.NoError(t, err)

config := Config{}
config.ApplyDefaults()
tf := NewTransformer(lgger, emitter, config)
_, pc, err := tf.Transform(giveReq)

require.NoError(t, err)
assert.Equal(t, defaultMaxTimeout, *pc.ModuleConfig.Timeout)
})

t.Run("tickInterval too high, applies default", func(t *testing.T) {
giveMap, err := values.NewMap(map[string]any{
"tickInterval": "1h",
"binary": []byte{0x01, 0x02, 0x03},
"config": []byte{0x04, 0x05, 0x06},
})
giveReq := capabilities.CapabilityRequest{
Config: giveMap,
}
require.NoError(t, err)

config := Config{}
config.ApplyDefaults()
tf := NewTransformer(lgger, emitter, config)
_, pc, err := tf.Transform(giveReq)

require.NoError(t, err)
assert.Equal(t, defaultMaxTickInterval, pc.ModuleConfig.TickInterval)
})

t.Run("applies default tick interval if missing", func(t *testing.T) {
giveMap, err := values.NewMap(map[string]any{
"binary": []byte{0x01, 0x02, 0x03},
"config": []byte{0x04, 0x05, 0x06},
})
giveReq := capabilities.CapabilityRequest{
Config: giveMap,
}
require.NoError(t, err)

config := Config{}
config.ApplyDefaults()
tf := NewTransformer(lgger, emitter, config)
_, pc, err := tf.Transform(giveReq)

require.NoError(t, err)
assert.Equal(t, defaultMaxTickInterval, pc.ModuleConfig.TickInterval)
})
}
7 changes: 7 additions & 0 deletions core/capabilities/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,15 @@ func unmarshalCapabilityConfig(data []byte) (capabilities.CapabilityConfiguratio
return capabilities.CapabilityConfiguration{}, err
}

rc, err := values.FromMapValueProto(cconf.RestrictedConfig)
if err != nil {
return capabilities.CapabilityConfiguration{}, err
}

return capabilities.CapabilityConfiguration{
DefaultConfig: dc,
RestrictedKeys: cconf.RestrictedKeys,
RestrictedConfig: rc,
RemoteTriggerConfig: remoteTriggerConfig,
RemoteTargetConfig: remoteTargetConfig,
}, nil
Expand Down
2 changes: 1 addition & 1 deletion core/scripts/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ require (
github.com/prometheus/client_golang v1.20.5
github.com/shopspring/decimal v1.4.0
github.com/smartcontractkit/chainlink-automation v0.8.1
github.com/smartcontractkit/chainlink-common v0.4.2-0.20250116214855-f49c5c27db51
github.com/smartcontractkit/chainlink-common v0.4.2-0.20250117101554-1922eef0bdd4
github.com/smartcontractkit/chainlink-data-streams v0.1.1-0.20250115135646-ac859d85e7e3
github.com/smartcontractkit/chainlink-testing-framework/lib v1.50.13
github.com/smartcontractkit/libocr v0.0.0-20241223215956-e5b78d8e3919
Expand Down
4 changes: 2 additions & 2 deletions core/scripts/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1164,8 +1164,8 @@ github.com/smartcontractkit/chainlink-ccip v0.0.0-20250120130359-cc025272bbff h1
github.com/smartcontractkit/chainlink-ccip v0.0.0-20250120130359-cc025272bbff/go.mod h1:JJZMCB75aVSAiPNW032F9WUKTlLztTd8bbQB5MEaZa4=
github.com/smartcontractkit/chainlink-ccip/chains/solana v0.0.0-20250103152858-8973fd0c912b h1:UBXi9Yj8YSMHDDaxQLu273x1fWjyEL9xP58nuJsqZfg=
github.com/smartcontractkit/chainlink-ccip/chains/solana v0.0.0-20250103152858-8973fd0c912b/go.mod h1:Bmwq4lNb5tE47sydN0TKetcLEGbgl+VxHEWp4S0LI60=
github.com/smartcontractkit/chainlink-common v0.4.2-0.20250116214855-f49c5c27db51 h1:YdjQiEu5uHWM1ApwdV+nLyJmu1+tt3IeiwPKNGoXwBI=
github.com/smartcontractkit/chainlink-common v0.4.2-0.20250116214855-f49c5c27db51/go.mod h1:yti7e1+G9hhkYhj+L5sVUULn9Bn3bBL5/AxaNqdJ5YQ=
github.com/smartcontractkit/chainlink-common v0.4.2-0.20250117101554-1922eef0bdd4 h1:cf7mgbR8OelUnq49x0vYLy1XWddw4t1Q1YsBPxUQY4M=
github.com/smartcontractkit/chainlink-common v0.4.2-0.20250117101554-1922eef0bdd4/go.mod h1:yti7e1+G9hhkYhj+L5sVUULn9Bn3bBL5/AxaNqdJ5YQ=
github.com/smartcontractkit/chainlink-cosmos v0.5.2-0.20241202195413-82468150ac1e h1:PRoeby6ZlTuTkv2f+7tVU4+zboTfRzI+beECynF4JQ0=
github.com/smartcontractkit/chainlink-cosmos v0.5.2-0.20241202195413-82468150ac1e/go.mod h1:mUh5/woemsVaHgTorA080hrYmO3syBCmPdnWc/5dOqk=
github.com/smartcontractkit/chainlink-data-streams v0.1.1-0.20250115135646-ac859d85e7e3 h1:GcPYNVFYjB065CNq0h8nK/VeU08nUkHgBX0cJIEpuHY=
Expand Down
1 change: 0 additions & 1 deletion core/services/gateway/network/httpclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ func NewHTTPClient(config HTTPClientConfig, lggr logger.Logger) (HTTPClient, err
SetBlockedIPs(config.BlockedIPs...).
SetBlockedIPsCIDR(config.BlockedIPsCIDR...).
SetCheckRedirect(disableRedirects).
EnableDebugLogging(true).
Build()

return &httpClient{
Expand Down
40 changes: 29 additions & 11 deletions core/services/workflows/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -833,14 +833,34 @@ func (e *Engine) workerForStepRequest(ctx context.Context, msg stepRequest) {
l.Debugf("sent step state update for execution %s with status %s", stepState.ExecutionID, stepStatus)
}

func merge(baseConfig *values.Map, overrideConfig *values.Map) *values.Map {
func merge(baseConfig *values.Map, capConfig capabilities.CapabilityConfiguration) *values.Map {
restrictedKeys := map[string]bool{}
for _, k := range capConfig.RestrictedKeys {
restrictedKeys[k] = true
}

// Shallow copy the defaults set in the onchain capability config.
m := values.EmptyMap()

if capConfig.DefaultConfig != nil {
for k, v := range capConfig.DefaultConfig.Underlying {
m.Underlying[k] = v
}
}

// Add in user-provided config, but skipping any restricted keys
for k, v := range baseConfig.Underlying {
m.Underlying[k] = v
if !restrictedKeys[k] {
m.Underlying[k] = v
}
}

if capConfig.RestrictedConfig == nil {
return m
}

for k, v := range overrideConfig.Underlying {
// Then overwrite the config with any restricted settings.
for k, v := range capConfig.RestrictedConfig.Underlying {
m.Underlying[k] = v
}

Expand Down Expand Up @@ -901,14 +921,12 @@ func (e *Engine) configForStep(ctx context.Context, lggr logger.Logger, step *st
return config, nil
}

if capConfig.DefaultConfig == nil {
return config, nil
}

// Merge the configs with registry config overriding the step config. This is because
// some config fields are sensitive and could affect the safe running of the capability,
// so we avoid user provided values by overriding them with config from the capabilities registry.
return merge(config, capConfig.DefaultConfig), nil
// Merge the capability registry config with the config provided by the user.
// We need to obey the following rules:
// - Remove any restricted keys
// - Overlay any restricted config
// - Merge the other keys, with user keys taking precedence
return merge(config, capConfig), nil
}

// executeStep executes the referenced capability within a step and returns the result.
Expand Down
Loading

0 comments on commit 6facc69

Please sign in to comment.