Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CRE-44] Add restricted configuration; validate transformer values #15961

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.1-0.20250115094325-4e61572bb9bd
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-20250110181647-9dba278f2103 h1
github.com/smartcontractkit/chainlink-ccip v0.0.0-20250110181647-9dba278f2103/go.mod h1:ncjd6mPZSRlelEqH/2KeLE1pU3UlqzBSn8RYkEoECzY=
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.1-0.20250115094325-4e61572bb9bd h1:SX16W7pqXGyn6fHFgtlr/rUdLZzBtQ5O3Gt3a6sFL70=
github.com/smartcontractkit/chainlink-common v0.4.1-0.20250115094325-4e61572bb9bd/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
Loading