Skip to content

Commit ba2d7f2

Browse files
committed
Apply Copilot PR feedback
1 parent 2b22671 commit ba2d7f2

10 files changed

Lines changed: 81 additions & 23 deletions

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ require (
4545
github.com/smartcontractkit/chain-selectors v1.0.100
4646
github.com/smartcontractkit/chainlink-common/pkg/chipingress v0.0.11-0.20260528204832-58c7145c53f8
4747
github.com/smartcontractkit/chainlink-protos/billing/go v0.0.0-20251024234028-0988426d98f4
48-
github.com/smartcontractkit/chainlink-protos/cre/go v0.0.0-20260622134419-a97fce3dedf3
48+
github.com/smartcontractkit/chainlink-protos/cre/go v0.0.0-20260622152157-c8e129347b8b
4949
github.com/smartcontractkit/chainlink-protos/linking-service/go v0.0.0-20251002192024-d2ad9222409b
5050
github.com/smartcontractkit/chainlink-protos/node-platform v0.0.0-20260205130626-db2a2aab956b
5151
github.com/smartcontractkit/chainlink-protos/storage-service v0.3.0

go.sum

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/workflows/host/execution_restrictions.go

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func (e *executionRestrictionsWithRawSecrets) GetRawSecrets(ctx context.Context,
9393
var _ ExecutionHelperWithRawSecrets = (*executionRestrictionsWithRawSecrets)(nil)
9494

9595
// NewRestrictedExecutionHelper wraps ExecutionHelper with restriction enforcement derived from r.
96-
// If r implements ExecutionHelperWithRawSecrets, the returned value will as well.
96+
// If inner implements ExecutionHelperWithRawSecrets, the returned value will as well.
9797
// If r is nil, ExecutionHelper is returned unchanged.
9898
func NewRestrictedExecutionHelper(inner ExecutionHelper, r *sdk.Restrictions) ExecutionHelper {
9999
if r == nil {
@@ -158,21 +158,23 @@ func (e *executionRestrictions) reserveCapabilityCall(request *sdk.CapabilityReq
158158
return false
159159
}
160160

161-
switch request.Payload.MessageName() {
162-
case confHttpRequest:
163-
conf := &confidentialhttp.ConfidentialHTTPRequest{}
164-
if err := request.Payload.UnmarshalTo(conf); err != nil {
165-
return false
166-
}
167-
168-
secrets := conf.GetVaultDonSecrets()
169-
for _, secret := range secrets {
170-
if !e.reserveSecret(&sdk.SecretRequest{
171-
Id: secret.Key,
172-
Namespace: secret.Namespace,
173-
}) {
161+
if request.Payload != nil {
162+
switch request.Payload.MessageName() {
163+
case confHttpRequest:
164+
conf := &confidentialhttp.ConfidentialHTTPRequest{}
165+
if err := request.Payload.UnmarshalTo(conf); err != nil {
174166
return false
175167
}
168+
169+
secrets := conf.GetVaultDonSecrets()
170+
for _, secret := range secrets {
171+
if !e.reserveSecret(&sdk.SecretRequest{
172+
Id: secret.Key,
173+
Namespace: secret.Namespace,
174+
}) {
175+
return false
176+
}
177+
}
176178
}
177179
}
178180

pkg/workflows/host/execution_restrictions_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,20 @@ func TestRequirementSelectingModule_ConfidentialHTTPWithRestrictions(t *testing.
419419
assert.Same(t, want, got)
420420
})
421421

422+
t.Run("nil payload is not treated as confidential http and reaches inner", func(t *testing.T) {
423+
// A capability call sharing the confidential-http method id but carrying no
424+
// payload must skip the vault-secret reservation branch entirely (guarded by
425+
// request.Payload != nil) and fall through to the normal method check.
426+
inner := mocks.NewMockExecutionHelper(t)
427+
want := &sdk.CapabilityResponse{}
428+
inner.EXPECT().CallCapability(matches.AnyContext, mock.Anything).Return(want, nil)
429+
h := host.NewRestrictedExecutionHelper(inner, restrictions())
430+
431+
got, err := h.CallCapability(t.Context(), &sdk.CapabilityRequest{Id: "confhttp@1.0.0", Method: "Call"})
432+
require.NoError(t, err)
433+
assert.Same(t, want, got)
434+
})
435+
422436
t.Run("disallowed confidential http call is denied without calling inner", func(t *testing.T) {
423437
inner := mocks.NewMockExecutionHelper(t) // no expectations: inner must not be called
424438
h := host.NewRestrictedExecutionHelper(inner, restrictions())

pkg/workflows/host/module.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ type ExecutionHelperWithRawSecrets interface {
5656

5757
// RestrictionAwareModule allows the module to know of the user-enforced restrictions.
5858
// Enforcement by this module is NOT to be trusted by the host,
59-
// however a violation is considered an indicator of a serious issues, such as compromise
59+
// however a violation is considered an indicator of a serious issue, such as compromise.
6060
type RestrictionAwareModule interface {
6161
Module
6262

pkg/workflows/host/requirement_selecting_module.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,12 @@ func (r *requirementSelectingModule) trigger(ctx context.Context, request *sdk.E
128128
if err != nil {
129129
return nil, fmt.Errorf("pre-hook execution failed: %w", err)
130130
}
131+
132+
switch preHookResult.Result.(type) {
133+
case *sdk.ExecutionResult_Error:
134+
return preHookResult, nil
135+
}
136+
131137
restrictions := preHookResult.GetRestrictions()
132138

133139
handler = NewRestrictedExecutionHelper(handler, restrictions)

pkg/workflows/host/requirement_selecting_module_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,43 @@ func TestRequirementSelectingModule_PreHook(t *testing.T) {
598598
assert.True(t, isRestricted, "additional module should receive a restricted helper")
599599
})
600600

601+
t.Run("pre-hook error result is returned directly without running the trigger", func(t *testing.T) {
602+
errResult := &sdk.ExecutionResult{
603+
Result: &sdk.ExecutionResult_Error{Error: "denied by pre-hook"},
604+
}
605+
606+
var helperSeenByAdditional ExecutionHelper
607+
main := ModuleAndHandler{Module: &stubModule{
608+
executeFn: func(_ context.Context, req *sdk.ExecuteRequest, _ ExecutionHelper) (*sdk.ExecutionResult, error) {
609+
if _, ok := req.Request.(*sdk.ExecuteRequest_PreHook); ok {
610+
return errResult, nil
611+
}
612+
return subscribeResult(subWithReqsAndPreHook(teeReqs)), nil
613+
},
614+
}}
615+
add := ModuleAndHandler{
616+
Module: &stubModule{
617+
executeFn: func(_ context.Context, _ *sdk.ExecuteRequest, h ExecutionHelper) (*sdk.ExecutionResult, error) {
618+
helperSeenByAdditional = h
619+
t.Fatal("additional module should not be called when pre-hook returns an error result")
620+
return nil, nil
621+
},
622+
},
623+
RequirementsHandler: RequirementsHandler{Tee: func(context.Context, *sdk.Tee) bool { return true }},
624+
}
625+
626+
m := NewRequirementSelectingModule(main, []ModuleAndHandler{add})
627+
m.Start()
628+
629+
_, err := m.Execute(t.Context(), subscribeRequest(), nil)
630+
require.NoError(t, err)
631+
632+
got, err := m.Execute(t.Context(), triggerRequest(0), &stubExecutionHelper{})
633+
require.NoError(t, err)
634+
assert.Same(t, errResult, got, "pre-hook error result should be returned unchanged")
635+
assert.Nil(t, helperSeenByAdditional, "additional module must not be invoked")
636+
})
637+
601638
t.Run("pre-hook error propagates", func(t *testing.T) {
602639
main := ModuleAndHandler{Module: &stubModule{
603640
executeFn: func(_ context.Context, req *sdk.ExecuteRequest, _ ExecutionHelper) (*sdk.ExecutionResult, error) {

pkg/workflows/host/tee_provider_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,12 @@ func TestNewTeeProvider(t *testing.T) {
137137
assert.False(t, provides(tee))
138138
})
139139

140-
t.Run("returns true when tee item is nil", func(t *testing.T) {
140+
t.Run("returns true when tee is nil", func(t *testing.T) {
141141
provides := NewTeeProvider(sdkpb.TeeType_TEE_TYPE_AWS_NITRO, []string{"us-west-2"})
142142
assert.True(t, provides(nil))
143143
})
144144

145-
t.Run("returns false when tee item is nil", func(t *testing.T) {
145+
t.Run("returns false when tee.Item item is nil", func(t *testing.T) {
146146
provides := NewTeeProvider(sdkpb.TeeType_TEE_TYPE_AWS_NITRO, []string{"us-west-2"})
147147
tee := &sdkpb.Tee{}
148148
assert.False(t, provides(tee))

pkg/workflows/wasm/host/module.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -682,7 +682,7 @@ func runWasm[I, O proto.Message](
682682
1, // memories
683683
)
684684

685-
deadline := *m.cfg.Timeout / m.cfg.TickInterval
685+
deadline := maxTimeout / m.cfg.TickInterval
686686
store.SetEpochDeadline(uint64(deadline))
687687

688688
h := fnv.New64a()
@@ -744,7 +744,7 @@ func runWasm[I, O proto.Message](
744744
// Note - there is no other reliable signal on the error that can be used to infer it is due to epoch deadline
745745
// being reached, so if an error is returned after the deadline it is assumed it is due to that and return
746746
// context.DeadlineExceeded.
747-
if err != nil && ((executionDuration >= *m.cfg.Timeout-m.cfg.TickInterval) || ctx.Err() != nil) { // As start could be called just before epoch update 1 tick interval is deducted to account for this
747+
if err != nil && ((executionDuration >= maxTimeout-m.cfg.TickInterval) || ctx.Err() != nil) { // As start could be called just before epoch update 1 tick interval is deducted to account for this
748748
m.cfg.Logger.Errorw("start function returned error after deadline reached, returning deadline exceeded error", "errFromStartFunction", err)
749749
return o, context.DeadlineExceeded
750750
}

pkg/workflows/wasm/host/standard_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -706,7 +706,6 @@ func makeTestModuleByName(t *testing.T, testPath, testName string, cfg *ModuleCo
706706

707707
cmd := exec.Command("make", wasmName) // #nosec
708708
cmd.Dir = absPath
709-
fmt.Printf("Compiling test module from %s with command %s\n:", cmd.Dir, cmd.String())
710709

711710
output, err := cmd.CombinedOutput()
712711
require.NoError(t, err, string(output))

0 commit comments

Comments
 (0)