Skip to content

[CRE] [5/5] Wire confidential workflow execution into CRE#21642

Open
nadahalli wants to merge 18 commits intotejaswi/cw-2-relay-handlerfrom
tejaswi/cw-5-wire-and-tests-v2
Open

[CRE] [5/5] Wire confidential workflow execution into CRE#21642
nadahalli wants to merge 18 commits intotejaswi/cw-2-relay-handlerfrom
tejaswi/cw-5-wire-and-tests-v2

Conversation

@nadahalli
Copy link
Copy Markdown
Contributor

Context

Part of #21635 (confidential workflow execution). [5/5] in the series.
Final wiring PR.

What this does

Wires PRs 1-4 together:

  • cre.go: starts confidential relay service when config is enabled
  • models.go: Attributes field on WorkflowSpec
  • System tests: ConfidentialRelay feature plugin, workflow registration
    with attributes, CompileAndDeployConfidentialWorkflow helper

This branch includes code from PRs 1-4 for compilation. As those merge
into develop, rebasing this branch will shrink the diff to just the
wiring and system test changes.

Dependencies

Depends on #21638, #21639, #21640, #21641 (PRs 1-4). Merge those first.

Copilot AI review requested due to automatic review settings March 23, 2026 17:01
@nadahalli nadahalli requested review from a team as code owners March 23, 2026 17:01
@github-actions
Copy link
Copy Markdown
Contributor

👋 nadahalli, thanks for creating this pull request!

To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team.

Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 23, 2026

CORA - Analysis Skipped

Reason: The number of code owners (4) is less than the minimum required (5) and/or the number of CODEOWNERS entries with changed files (8) is less than the minimum required (2).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 23, 2026

✅ No conflicts with other open PRs targeting tejaswi/cw-2-relay-handler

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Risk Rating: HIGH

Wires confidential workflow execution into CRE end-to-end (registry attributes → syncer routing → confidential module execution → gateway relay plumbing), with accompanying config, DB migration, and system-test helpers/plugins.

Changes:

  • Add confidential workflow execution support via WorkflowSpec.Attributes, confidential engine routing, and ConfidentialModule.
  • Introduce gateway + relay DON plumbing (new gateway handler type + CRE subservice) and related config.
  • Extend system tests to register workflows with attributes and provide a confidential relay feature/plugin.

Targeted areas for scrupulous human review:

  • DB migration for workflow_specs_v2.attributes and the ORM upsert path.
  • Filesystem fetcher path validation changes (security boundary check).
  • Confidential relay request/attestation verification flow and error handling (gateway handler + relay handler).

Recommended reviewers (per CODEOWNERS):

  • @smartcontractkit/keystone (core/services/workflows, core/capabilities, deployment/cre)
  • @smartcontractkit/capabilities-team (core/capabilities)
  • @smartcontractkit/operations-platform (deployment/cre)

Reviewed changes

Copilot reviewed 40 out of 45 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
system-tests/tests/test-helpers/t_helpers.go Updates workflow artifact creation/registration, adds attributes + confidential deploy helper; currently removes helpers still used elsewhere.
system-tests/tests/go.mod Bumps deps to support confidential workflow/attestation work.
system-tests/tests/go.sum go.sum updates for system-tests/tests module.
system-tests/lib/go.mod Bumps deps to support confidential workflow/attestation work.
system-tests/lib/go.sum go.sum updates for system-tests/lib module.
system-tests/lib/cre/workflow/workflow.go Extends workflow registration to include attributes.
system-tests/lib/cre/types.go Adds ConfidentialRelayCapability flag.
system-tests/lib/cre/features/confidential_relay/confidential_relay.go New system-test feature plugin wiring confidential relay handler + node TOML overrides.
core/store/migrate/migrations/0295_add_workflow_attributes_column.sql Adds attributes column to workflow_specs_v2 (migration default needs fix).
core/services/job/models.go Adds WorkflowSpec.Attributes DB field.
core/services/workflows/artifacts/v2/orm.go Persists attributes in workflow spec upsert.
core/services/workflows/syncer/v2/handler.go Routes confidential workflows to confidential engine creation path.
core/services/workflows/syncer/v2/handler_test.go Adds tests for confidential vs non-confidential routing + malformed attributes.
core/services/workflows/v2/confidential_module.go New module delegating execution to confidential-workflows capability.
core/services/workflows/v2/confidential_module_test.go Unit tests for attribute parsing, hashing, and module execution request/response handling.
core/services/workflows/v2/engine.go Adjusts execution timestamp handling and tracing attributes.
core/services/workflows/v2/capability_executor.go Changes execution timestamp representation in helper.
core/services/workflows/syncer/v2/fetcher.go Adapts file fetcher to support HTTP(S) URLs for confidential workflows (path boundary check needs hardening).
core/services/workflows/syncer/fetcher.go Same fetcher change for non-v2 path (path boundary check needs hardening).
core/services/gateway/handlers/confidentialrelay/handler.go New gateway handler implementing confidential relay fanout/quorum aggregation and callbacks.
core/services/gateway/handlers/confidentialrelay/handler_test.go Tests for the new gateway handler behavior (quorum, rate limiting, timeouts, etc.).
core/services/gateway/handlers/confidentialrelay/aggregator.go Aggregator implementing F+1 quorum by response digest.
core/services/gateway/handler_factory.go Registers the new confidential-compute-relay handler type.
deployment/cre/jobs/pkg/gateway_job.go Adds gateway job wiring for confidential-compute-relay handler type and service name mapping.
core/capabilities/confidentialrelay/service.go New CRE subservice wrapper that starts the confidential relay handler when enabled.
core/capabilities/confidentialrelay/handler.go New relay DON gateway-connector handler: attestation validation + proxy to vault/capabilities.
core/capabilities/confidentialrelay/handler_test.go Unit tests for relay DON handler request processing and lifecycle.
core/services/cre/cre.go Starts confidential relay service when CRE config enables it.
core/config/cre_config.go Adds CREConfidentialRelay interface to config surface.
core/services/chainlink/config_cre.go Implements ConfidentialRelay() config adapter for CRE config.
core/config/toml/types.go Adds TOML config struct for CRE.ConfidentialRelay and merge logic.
core/services/standardcapabilities/conversions/conversions.go Adds “mock” mapping for capability ID/command conversions.
core/capabilities/launcher.go Logger type changes + single-DON capability serving fix + cross-DON capability discovery for capability DONs.
core/capabilities/launcher_test.go Adds regression test for single-DON capability serving + adjusts mock expectations.
core/services/workflows/models.go Adds a field on step struct (supports workflow execution changes).
core/services/workflows/metering/metering.go Logger interface change + reduces trace logging to a single debug log.
core/services/workflows/syncer/engine_registry_test.go Minor test tweak (adds const).
core/scripts/cre/environment/environment/workflow.go Updates workflow registration call to pass attributes argument.
go.mod Bumps deps (cbor + chainlink-common + teeattestation + nitrite).
go.sum go.sum updates for root module.
deployment/go.mod Bumps deps for deployment module (chainlink-common/protos, cbor, teeattestation, nitrite).
deployment/go.sum go.sum updates for deployment module.
core/scripts/go.mod Bumps deps for scripts module (chainlink-common/protos, teeattestation, nitrite).
core/scripts/go.sum go.sum updates for scripts module.
.changeset/confidential-workflow-execution.md Adds changelog entry for confidential workflow execution feature.
Comments suppressed due to low confidence (2)

core/services/workflows/syncer/v2/fetcher.go:231

  • The path containment check uses strings.HasPrefix(fullPath, basePath), which is not a safe filesystem boundary check (e.g., basePath /tmp/base will match /tmp/base2/...). Use filepath.Rel (reject any rel path starting with ..) or ensure fullPath is cleaned/absolute and compare against basePath+string(os.PathSeparator).
		if !strings.HasPrefix(fullPath, basePath) {
			return nil, fmt.Errorf("request URL %s is not within the basePath %s", fullPath, basePath)
		}

core/services/workflows/syncer/fetcher.go:197

  • The path containment check uses strings.HasPrefix(fullPath, basePath), which is not a safe filesystem boundary check (e.g., basePath /tmp/base will match /tmp/base2/...). Use filepath.Rel (reject any rel path starting with ..) or ensure fullPath is cleaned/absolute and compare against basePath+string(os.PathSeparator).
		if !strings.HasPrefix(fullPath, basePath) {
			return nil, fmt.Errorf("request URL %s is not within the basePath %s", fullPath, basePath)
		}

@@ -0,0 +1,5 @@
-- +goose Up
ALTER TABLE workflow_specs_v2 ADD COLUMN attributes bytea DEFAULT '';
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This migration sets a bytea column default to a text literal (DEFAULT ''), which will fail on Postgres with a type mismatch. Use an explicit bytea default (e.g., ''::bytea or '\x'::bytea) or make the column nullable with no default and handle empty attributes in application code.

Copilot uses AI. Check for mistakes.
Comment on lines +635 to +636
require.IsType(t, &evm.Blockchain{}, blockchains[0], "expected EVM blockchain type")
deleteErr := creworkflow.DeleteWithContract(t.Context(), blockchains[0].(*evm.Blockchain).SethClient, workflowRegistryAddress, version, uniqueWorkflowName)
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleteWorkflows indexes blockchains[0] without checking length; if the environment has zero blockchains this will panic before the type assertion. Add a length check (and ideally pick the registry chain explicitly rather than assuming index 0).

Copilot uses AI. Check for mistakes.
Comment on lines +683 to +688
// CompileAndDeployConfidentialWorkflow compiles a workflow WASM binary, copies it to Docker
// containers, and registers it with confidential attributes on the workflow registry.
func CompileAndDeployConfidentialWorkflow[T WorkflowConfig](t *testing.T,
testEnv *ttypes.TestEnvironment, testLogger zerolog.Logger, workflowName string,
workflowConfig *T, workflowFileLocation string, attributes []byte,
) string {
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UniqueWorkflowName and ParallelEnabled were removed from this helper, but they are still referenced by multiple system tests (e.g., system-tests/tests/smoke/cre/cre_suite_test.go, system-tests/tests/regression/cre/cre_regression_suite_test.go). This will break compilation for the system-tests module unless the call sites are updated or these helpers are reintroduced.

Copilot uses AI. Check for mistakes.
@nadahalli nadahalli requested a review from a team as a code owner March 23, 2026 17:19
@nadahalli nadahalli force-pushed the tejaswi/cw-5-wire-and-tests-v2 branch from f652007 to 571766f Compare March 23, 2026 17:26
@trunk-io
Copy link
Copy Markdown

trunk-io bot commented Mar 23, 2026

Static BadgeStatic BadgeStatic BadgeStatic Badge

Failed Test Failure Summary Logs
TestScripts/node/validate/disk-based-logging-disabled The test failed during validation of node disk-based logging configuration, likely due to incorrect or disabled logging settings. Logs ↗︎
TestScripts/node/validate/disk-based-logging-disabled The test failed without a specific error message, indicating an unspecified failure during script execution. Logs ↗︎
TestScripts/node/validate/defaults-override The test failed during a validation command, but the specific cause of failure is not indicated in the log. Logs ↗︎
TestScripts/node/validate/defaults-override The test failed during environment validation, but the specific cause of failure is not indicated in the log. Logs ↗︎

... and 26 more

View Full Report ↗︎Docs

@nadahalli nadahalli force-pushed the tejaswi/cw-5-wire-and-tests-v2 branch from 81fce03 to 898b32b Compare March 23, 2026 19:08
@@ -0,0 +1,484 @@
package confidentialrelay
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this file from a prior PR?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this PR has not perfectly caught the rest of the chain.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, from #21639 [2/5]. Included here for compilation. Will disappear from this PR's diff after #21639 merges.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Rebuilt PR from scratch using the clean PR 1-4 branches. All core-test drift removed.

Comment on lines +40 to +41
trustedPCRs: trustedPCRs,
caRootsPEM: caRootsPEM,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, don't really think either of these need to be constructor-level params so long as we have the capRegistry param.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Updated NewService to pull config from capabilities registry. cre.go now calls NewService(wrapper, capRegistry, lggr) with no trustedPCRs/caRootsPEM params.

type launcher struct {
services.StateMachine
lggr logger.SugaredLogger
lggr logger.Logger
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably don't want to mess with this, here or anywhere else below.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Removed logger type changes and allWorkflowDONs fix (redundant with LocalOnly). Only the addRemoteCapabilities fix for capability DONs remains (PR #21640).

if w.don2donSharedPeer != nil {
donPairs := w.donPairsToUpdate(w.myPeerID, localRegistry)
err := w.don2donSharedPeer.UpdateConnectionsByDONs(ctx, donPairs, w.p2pStreamConfig)
err := w.don2donSharedPeer.UpdateConnectionsByDONs(ctx, donPairs, defaultStreamConfig)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the addRemoteCapabilities fix from #21640 [3/5]. Capability DONs need to discover remote capabilities (relay DON calling vault DON). Will disappear after #21640 merges.

balance *balanceStore
client BillingClient
lggr logger.SugaredLogger
lggr logger.Logger
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file shouldn't change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

func TestEngineRegistry(t *testing.T) {
var srv services.Service = &fakeService{}

const id1 = "foo"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mistake

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

*Engine
WorkflowExecutionID string
ExecutionTimestamp time.Time
ExecutionTimestamp int64
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure this should be touched

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

Comment on lines -595 to -603
creCtx := contexts.CREValue(ctx)
// Tracer is no-op if DebugMode is false
ctx, span := e.tracer.Start(ctx, "workflow_execution",
trace.WithAttributes(
attribute.String("workflow_name", e.cfg.WorkflowName.String()),
attribute.String("version", "v2"),
attribute.String("org_id", creCtx.Org),
attribute.String("owner_id", creCtx.Owner),
attribute.String("workflow_id", creCtx.Workflow),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is any of this changing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

}
e.metrics.IncrementExecutionIDLegacyCounter(ctx)
}
trace.SpanFromContext(ctx).SetAttributes(attribute.String("execution_id", executionID))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think any bit of this file should change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

workflows.Vertex
capability capabilities.ExecutableCapability
info capabilities.CapabilityInfo
config *values.Map
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

concerning to see a change like this. Why do we need this added to the step struct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed. This was core-test drift, not part of the confidential workflow changes.

nadahalli added a commit that referenced this pull request Mar 26, 2026
The enclave needs an authenticated URL to download WASM binaries from
the CRE storage service. BinaryURLResolver resolves the raw on-chain
URL into a presigned/ephemeral URL per execution. Nil-safe: falls
back to the raw URL when no resolver is set.

PR 5/5 (#21642) wires this to the storage service retriever.
nadahalli added a commit that referenced this pull request Mar 27, 2026
The enclave needs an authenticated URL to download WASM binaries from
the CRE storage service. BinaryURLResolver resolves the raw on-chain
URL into a presigned/ephemeral URL per execution. Nil-safe: falls
back to the raw URL when no resolver is set.

PR 5/5 (#21642) wires this to the storage service retriever.
@nadahalli nadahalli force-pushed the tejaswi/cw-5-wire-and-tests-v2 branch from 16e4fc9 to 5cfc03f Compare March 31, 2026 16:24
@nadahalli nadahalli changed the base branch from develop to tejaswi/cw-2-relay-handler March 31, 2026 16:29
nadahalli added a commit that referenced this pull request Apr 1, 2026
The enclave needs an authenticated URL to download WASM binaries from
the CRE storage service. BinaryURLResolver resolves the raw on-chain
URL into a presigned/ephemeral URL per execution. Nil-safe: falls
back to the raw URL when no resolver is set.

PR 5/5 (#21642) wires this to the storage service retriever.
@nadahalli nadahalli force-pushed the tejaswi/cw-5-wire-and-tests-v2 branch from 5cfc03f to cd40794 Compare April 1, 2026 11:04
nadahalli added a commit that referenced this pull request Apr 1, 2026
The enclave needs an authenticated URL to download WASM binaries from
the CRE storage service. BinaryURLResolver resolves the raw on-chain
URL into a presigned/ephemeral URL per execution. Nil-safe: falls
back to the raw URL when no resolver is set.

PR 5/5 (#21642) wires this to the storage service retriever.
nadahalli added a commit that referenced this pull request Apr 1, 2026
The enclave needs an authenticated URL to download WASM binaries from
the CRE storage service. BinaryURLResolver resolves the raw on-chain
URL into a presigned/ephemeral URL per execution. Nil-safe: falls
back to the raw URL when no resolver is set.

PR 5/5 (#21642) wires this to the storage service retriever.
@nadahalli nadahalli force-pushed the tejaswi/cw-2-relay-handler branch from cb65a31 to 00a16e6 Compare April 2, 2026 14:12
nadahalli and others added 18 commits April 2, 2026 16:21
Wires PRs 1-4 together:

- cre.go: start confidential relay service when gateway connector exists
- system tests: ConfidentialRelay feature plugin, workflow registration
  with attributes, CompileAndDeployConfidentialWorkflow helper

This branch includes code from PRs 1-4 for compilation. As those merge
into develop, rebasing this branch will shrink the diff to just the
wiring and system test changes.

Part of #21635
The enclave needs an authenticated URL to download WASM binaries from
the CRE storage service. BinaryURLResolver resolves the raw on-chain
URL into a presigned/ephemeral URL per execution. Nil-safe: falls
back to the raw URL when no resolver is set.

PR 5/5 (#21642) wires this to the storage service retriever.
…CC plugin ref

- chainlink-common v0.11.2-0.20260330123353-c1870a5eff52 (teeattestation merged into root)
- Pavel review fixes: sendResponseAndCleanup, error sanitization, requestTimeoutSec
- CC plugin ref updated to 526a251
(cherry picked from commit 7b4af73)
(cherry picked from commit 9a721064b9ca7d070231a0031b33d1db82b16eef)
nadahalli added a commit that referenced this pull request Apr 2, 2026
The enclave needs an authenticated URL to download WASM binaries from
the CRE storage service. BinaryURLResolver resolves the raw on-chain
URL into a presigned/ephemeral URL per execution. Nil-safe: falls
back to the raw URL when no resolver is set.

PR 5/5 (#21642) wires this to the storage service retriever.
@nadahalli nadahalli force-pushed the tejaswi/cw-5-wire-and-tests-v2 branch from 0603953 to b22f1da Compare April 2, 2026 14:26
@cl-sonarqube-production
Copy link
Copy Markdown

Quality Gate failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarQube IDE SonarQube IDE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants