[CRE] [4/5] ConfidentialModule, config, DB migration, syncer routing#21641
[CRE] [4/5] ConfidentialModule, config, DB migration, syncer routing#21641
Conversation
|
👋 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! |
|
I see you updated files related to
|
|
✅ No conflicts with other open PRs targeting |
There was a problem hiding this comment.
Pull request overview
Risk Rating: HIGH
This PR adds the core plumbing for “confidential workflow execution” by persisting on-chain workflow attributes, introducing a ConfidentialModule implementation that delegates execution to the confidential-workflows capability, and routing confidential workflows onto that path. It also introduces gateway + node-side confidential relay handlers and configuration needed for relay DON participation.
Changes:
- Persist workflow
Attributes(DB + ORM + model) and route confidential workflows to a confidential engine creation path. - Add
ConfidentialModule(host.ModuleV2) that dispatches execution to theconfidential-workflowscapability, including secret identifiers forwarding. - Add confidential relay handler implementations (gateway-side fanout + quorum aggregation; node-side attestation validation + Vault/capability proxy) plus CRE config surface.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Bumps deps needed for confidential workflow/relay support. |
| go.sum | Updates checksums for dependency changes. |
| core/store/migrate/migrations/0295_add_workflow_attributes_column.sql | Adds attributes column to workflow_specs_v2. |
| core/services/job/models.go | Adds WorkflowSpec.Attributes for DB persistence. |
| core/services/workflows/artifacts/v2/orm.go | Upsert now writes attributes to DB. |
| core/services/workflows/syncer/v2/handler.go | Persists attributes into specs; routes confidential workflows to confidential engine creation; refactors engine start/register helper. |
| core/services/workflows/syncer/v2/handler_test.go | Adds tests validating confidential vs non-confidential routing behavior. |
| core/services/workflows/v2/confidential_module.go | Implements ConfidentialModule, attributes parsing, and binary hashing for confidential execution. |
| core/services/workflows/v2/confidential_module_test.go | Unit tests for attribute parsing, hashing, and capability dispatch behavior. |
| core/services/workflows/syncer/fetcher.go | Extends file fetcher to accept HTTP(S) URLs for local reads (confidential workflow compatibility). |
| core/services/workflows/syncer/v2/fetcher.go | Same file-fetcher HTTP(S) URL support for v2 syncer. |
| core/services/standardcapabilities/conversions/conversions.go | Adds mock command/capability ID mapping. |
| core/services/gateway/handlers/confidentialrelay/handler.go | Adds gateway confidential relay handler (fanout, quorum aggregation, timeouts, metrics). |
| core/services/gateway/handlers/confidentialrelay/aggregator.go | Adds quorum aggregation logic based on response digest matching. |
| core/services/gateway/handlers/confidentialrelay/handler_test.go | Adds test coverage for gateway relay handler behaviors (quorum, timeouts, rate limiting, etc.). |
| core/config/cre_config.go | Extends CRE config interface with ConfidentialRelay. |
| core/config/toml/types.go | Adds [CRE.ConfidentialRelay] TOML block and merge behavior. |
| core/services/chainlink/config_cre.go | Implements CREConfidentialRelay accessors for runtime config. |
| core/capabilities/confidentialrelay/service.go | Adds lifecycle wrapper to create/start node-side relay handler once gateway connector is available. |
| core/capabilities/confidentialrelay/handler.go | Adds node-side enclave relay handler (attestation verification + Vault/capability proxy). |
| core/capabilities/confidentialrelay/handler_test.go | Adds tests for node-side relay handler behavior (cap exec, errors, lifecycle). |
Scrupulous human review recommended for:
core/services/workflows/syncer/v2/handler.go:tryEngineCreate/tryConfidentialEngineCreate/startAndRegisterEngineflow (engine lifecycle + hook wiring).core/capabilities/confidentialrelay/handler.go:verifyAttestationHash+ request translation into Vault/capability calls (security-sensitive).
Suggested reviewers (per .github/CODEOWNERS):
@smartcontractkit/keystone(workflows + capabilities areas)@smartcontractkit/capabilities-team(capabilities handlers)@smartcontractkit/foundations/@smartcontractkit/core(job models, config, deps)
| // For HTTP(S) URLs, extract just the filename and resolve against basePath. | ||
| // This supports confidential workflows where the on-chain URL must be HTTP | ||
| // (so the enclave can fetch the binary), but the syncer reads from the local filesystem. | ||
| fullPath = filepath.Join(basePath, filepath.Base(u.Path)) |
There was a problem hiding this comment.
For HTTP(S) URLs, collapsing the path to filepath.Base(u.Path) means different on-chain URLs that share the same last segment (e.g. /wfA/binary and /wfB/binary) will map to the same local file, so the syncer can read the wrong artifact. Consider preserving more of the URL path (e.g., clean u.Path, trim leading '/', and join to basePath) or otherwise deriving a collision-resistant local path, and add a unit test covering the HTTP(S)-URL-with-file-basePath behavior.
| // For HTTP(S) URLs, extract just the filename and resolve against basePath. | |
| // This supports confidential workflows where the on-chain URL must be HTTP | |
| // (so the enclave can fetch the binary), but the syncer reads from the local filesystem. | |
| fullPath = filepath.Join(basePath, filepath.Base(u.Path)) | |
| // For HTTP(S) URLs, preserve the cleaned path (minus any leading slash) and | |
| // resolve it against basePath. This avoids collisions where different URLs | |
| // share the same last segment (e.g., /wfA/binary and /wfB/binary). | |
| cleanPath := strings.TrimPrefix(filepath.Clean(u.Path), "/") | |
| fullPath = filepath.Clean(filepath.Join(basePath, cleanPath)) |
There was a problem hiding this comment.
Acknowledged. This is a dev-mode file fetcher, not a production path. In file-fetcher mode there's a single flat directory of artifacts. filepath.Base is safe here (no path traversal, existing hasPrefix check still runs). Filename collisions would already be a problem at the directory level.
There was a problem hiding this comment.
Reverted the entire HTTP URL handling from the file fetcher. The code path was dead; the enclave has its own binary fetcher.
| // For HTTP(S) URLs, extract just the filename and resolve against basePath. | ||
| // This supports confidential workflows where the on-chain URL must be HTTP | ||
| // (so the enclave can fetch the binary), but the syncer reads from the local filesystem. | ||
| fullPath = filepath.Join(basePath, filepath.Base(u.Path)) |
There was a problem hiding this comment.
For HTTP(S) URLs, collapsing the path to filepath.Base(u.Path) means different on-chain URLs that share the same last segment (e.g. /wfA/binary and /wfB/binary) will map to the same local file, so the syncer can read the wrong artifact. Consider preserving more of the URL path (e.g., clean u.Path, trim leading '/', and join to basePath) or otherwise deriving a collision-resistant local path, and add a unit test covering the HTTP(S)-URL-with-file-basePath behavior.
| // For HTTP(S) URLs, extract just the filename and resolve against basePath. | |
| // This supports confidential workflows where the on-chain URL must be HTTP | |
| // (so the enclave can fetch the binary), but the syncer reads from the local filesystem. | |
| fullPath = filepath.Join(basePath, filepath.Base(u.Path)) | |
| // For HTTP(S) URLs, preserve the URL path (relative to basePath) instead of | |
| // collapsing to just the filename. This avoids collisions where different | |
| // URLs share the same basename (e.g., /wfA/binary and /wfB/binary). | |
| cleanPath := filepath.Clean(u.Path) | |
| // Make the path relative so we always resolve it under basePath. | |
| cleanPath = strings.TrimPrefix(cleanPath, string(filepath.Separator)) | |
| if cleanPath == "" || cleanPath == "." { | |
| return nil, fmt.Errorf("invalid HTTP URL path %q", u.Path) | |
| } | |
| fullPath = filepath.Join(basePath, cleanPath) |
There was a problem hiding this comment.
Same as above, reverted.
| // Normalize owner to EIP-55 checksum format, matching how secrets are stored. | ||
| normalizedOwner := common.HexToAddress(params.Owner).Hex() | ||
|
|
||
| vaultReq := &vault.GetSecretsRequest{ | ||
| Requests: make([]*vault.SecretRequest, 0, len(params.Secrets)), | ||
| } | ||
| for _, s := range params.Secrets { | ||
| namespace := s.Namespace | ||
| if namespace == "" { | ||
| namespace = vaulttypes.DefaultNamespace | ||
| } | ||
| vaultReq.Requests = append(vaultReq.Requests, &vault.SecretRequest{ | ||
| Id: &vault.SecretIdentifier{ | ||
| Key: s.Key, | ||
| Namespace: namespace, | ||
| Owner: normalizedOwner, | ||
| }, | ||
| EncryptionKeys: []string{params.EnclavePublicKey}, | ||
| }) | ||
| } |
There was a problem hiding this comment.
handleSecretsGet forwards params.EnclavePublicKey into vaultReq.EncryptionKeys without validating it is present. If it's empty, VaultDON will either fail later or return data that translateVaultResponse can't match (leading to an internal error instead of a clear invalid-params response). Consider validating required fields (Owner, WorkflowID, ExecutionID, EnclavePublicKey, and secret identifiers) up front and returning jsonrpc.ErrInvalidParams for missing/empty values.
There was a problem hiding this comment.
This file has been removed from this PR. It belongs to #21639 [2/5], where the nil-params and owner-validation fixes have been applied.
CORA - Analysis SkippedReason: The number of code owners (3) is less than the minimum required (5) and/or the number of CODEOWNERS entries with changed files (4) is less than the minimum required (2). |
nadahalli
force-pushed
the
tejaswi/cw-4-confidential-module
branch
3 times, most recently
from
54fbf1c to
e7a3894
Compare
| CreatedAt time.Time `toml:"-"` | ||
| UpdatedAt time.Time `toml:"-"` | ||
| SpecType WorkflowSpecType `toml:"spec_type" db:"spec_type"` | ||
| Attributes []byte `db:"attributes"` |
There was a problem hiding this comment.
Let's figure out if we need a new Attributes field here or can overload a different existing field.
There was a problem hiding this comment.
We can probably open a thread with dev services today.
| if u.Scheme == "http" || u.Scheme == "https" { | ||
| // For HTTP(S) URLs, extract just the filename and resolve against basePath. | ||
| // This supports confidential workflows where the on-chain URL must be HTTP | ||
| // (so the enclave can fetch the binary), but the syncer reads from the local filesystem. |
There was a problem hiding this comment.
I imagine we don't want to use regular http schemes in prod.
There was a problem hiding this comment.
Is this meant to allow us to mock something in E2E tests? Since obviously the real syncer service will be remote over https.
We can do a self-signed/insecure https endpoint for our own tests and continue using a local filesystem-based fetcher.
There was a problem hiding this comment.
You're right. Reverted the HTTP URL handling from the file fetcher in both syncer/ and syncer/v2/. The enclave has its own BinaryFetcher that does the HTTP fetch independently; the node syncer doesn't need to handle HTTP URLs in the file fetcher path.
There was a problem hiding this comment.
Reverted. The file fetcher now only handles file paths, as before. The enclave fetches the binary over HTTP via its own fetcher (confidential-compute/enclave/apps/confidential-workflows/app/fetcher.go), which is completely separate from the node syncer path.
| // tryConfidentialEngineCreate creates a V2 engine backed by a ConfidentialModule | ||
| // instead of a local WASM module. The ConfidentialModule delegates execution to | ||
| // the confidential-workflows capability which runs the WASM inside a TEE. | ||
| func (h *eventHandler) tryConfidentialEngineCreate( |
There was a problem hiding this comment.
I think this function duplicates too much existing code that we don't want to maintain, so creates maintainability issues for others.
There was a problem hiding this comment.
Done. Unified the engine creation flow and inlined startAndRegisterEngine back into tryEngineCreate so the confidential and normal paths share the same code.
| cfg := &v2.EngineConfig{ | ||
| Lggr: h.lggr, | ||
| Module: module, | ||
| WorkflowConfig: []byte(spec.Config), | ||
| CapRegistry: h.capRegistry, | ||
| DonSubscriber: h.workflowDonSubscriber, | ||
| UseLocalTimeProvider: h.useLocalTimeProvider, | ||
| DonTimeStore: h.donTimeStore, | ||
| ExecutionsStore: h.workflowStore, | ||
| WorkflowID: spec.WorkflowID, | ||
| WorkflowOwner: spec.WorkflowOwner, | ||
| WorkflowName: workflowName, | ||
| WorkflowTag: spec.WorkflowTag, | ||
| WorkflowEncryptionKey: h.workflowEncryptionKey, | ||
|
|
||
| LocalLimits: v2.EngineLimits{}, | ||
| LocalLimiters: h.engineLimiters, | ||
| GlobalExecutionConcurrencyLimiter: h.workflowLimits, | ||
|
|
||
| BeholderEmitter: h.emitter, | ||
| BillingClient: h.billingClient, | ||
|
|
||
| WorkflowRegistryAddress: h.workflowRegistryAddress, | ||
| WorkflowRegistryChainSelector: h.workflowRegistryChainSelector, | ||
| OrgResolver: h.orgResolver, | ||
| SecretsFetcher: h.secretsFetcher, | ||
| FeatureFlags: h.featureFlags, | ||
| } | ||
|
|
||
| existingHook := cfg.Hooks.OnInitialized | ||
| cfg.Hooks.OnInitialized = func(err error) { | ||
| initDone <- err | ||
| if existingHook != nil { | ||
| existingHook(err) | ||
| } | ||
| } | ||
|
|
||
| engine, err := v2.NewEngine(cfg) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create confidential workflow engine: %w", err) | ||
| } | ||
|
|
||
| return h.startAndRegisterEngine(ctx, engine, initDone, wid, spec.WorkflowID, source) |
There was a problem hiding this comment.
all this is boilerplate, so would be nice to refactor this.
There was a problem hiding this comment.
Done. Extracted newV2EngineConfig and cleaned up factory signatures.
| // If it's not absolute, we assume it's relative to the basePath | ||
| fullPath = filepath.Join(basePath, fullPath) | ||
| var fullPath string | ||
| if u.Scheme == "http" || u.Scheme == "https" { |
There was a problem hiding this comment.
again, we should be able to set something up over https in our tests.
There was a problem hiding this comment.
Agreed, reverted.
| @@ -0,0 +1,188 @@ | |||
| package v2 | |||
nadahalli
force-pushed
the
tejaswi/cw-4-confidential-module
branch
from
9bbc5e5 to
f58cb63
Compare
|
|
||
| // newV2EngineConfig builds the common EngineConfig shared by both the normal | ||
| // WASM engine and the confidential engine paths. Caller supplies the module. | ||
| func (h *eventHandler) newV2EngineConfig( |
nadahalli
force-pushed
the
tejaswi/cw-4-confidential-module
branch
from
13931fe to
de37e16
Compare
nadahalli
force-pushed
the
tejaswi/cw-4-confidential-module
branch
2 times, most recently
from
ad18f1a to
a815ace
Compare
|
The filepath.Base() code for HTTP URLs in newFileFetcher was dead code. No test or production path sends an HTTP URL to a file-based fetcher. The enclave fetches binaries via its own BinaryFetcher, independent of the node syncer's fetcher.
Restore comments that were dropped when extracting startAndRegisterEngine from tryEngineCreate. Extract common EngineConfig construction into newV2EngineConfig and initDone hook wiring into wireInitDoneHook, reducing duplication between the normal and confidential engine paths.
Replace the early-return pattern with a symmetric if/else that picks the factory, then converges on a single startAndRegisterEngine call. Rename tryConfidentialEngineCreate to confidentialEngineFactory and change its signature to return (services.Service, error). # Conflicts: # core/services/workflows/syncer/v2/handler.go
No longer needed as a separate method now that both engine paths converge in tryEngineCreate.
Group string params together in newV2EngineConfig, move SdkName and DebugMode into the constructor, drop unused wid param from confidentialEngineFactory. # Conflicts: # core/services/workflows/syncer/v2/handler.go
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
force-pushed
the
tejaswi/cw-4-confidential-module
branch
from
a815ace to
764834e
Compare
Context
Part of #21635 (confidential workflow execution). [4/5] in the series.
Can be reviewed and merged independently.
What this does
Core abstractions for confidential workflow execution:
host.ModuleV2, dispatches workflowexecution to TEE enclave via the
confidential-workflowscapabilityinstead of running WASM locally. Strategy pattern; the engine is
unchanged.
tryEngineCreatechecksIsConfidential(spec.Attributes).If true, short-circuits to
tryConfidentialEngineCreate. 6-lineearly-return, existing path untouched.
attributes byteacolumn toworkflow_specs_v2.Nothing is wired into CRE yet. The routing is inert until PR 5/5.
Dependencies
None. Compiles against develop independently.