[CRE] Support confidential workflow engine path#21443
Closed
[CRE] Support confidential workflow engine path#21443
Conversation
Include myWorkflowDONs when passing workflow DONs to serveCapabilities. In single-DON topologies (e.g. local CRE), the same DON acts as both the workflow DON and the capability DON. The launcher classified it into myWorkflowDONs (not remoteWorkflowDONs), so remoteWorkflowDONs was empty. Passing only remoteWorkflowDONs to serveCapabilities caused executable/server.go to reject the capability with "empty workflowDONs provided".
Covers the topology where a single DON is both a workflow DON and a capability DON (e.g. local CRE). Verifies that capabilities are served correctly when remoteWorkflowDONs is empty but myWorkflowDONs is not.
…lows - DB migration 0291: add attributes bytea column to workflow_specs_v2 - WorkflowSpec: add Attributes field, persist through ORM - Handler: store payload.Attributes, route confidential workflows to dedicated engine creation path (tryConfidentialEngineCreate) - ConfidentialModule: host.ModuleV2 impl that delegates execution to the confidential-workflows@1.0.0-alpha capability via CapabilitiesRegistry - Plugin registration for confidential-workflows in plugins.private.yaml
- IsConfidential now returns (bool, error) instead of silently swallowing malformed attributes JSON - Add info log when routing workflow to confidential execution - Add unit tests for ParseWorkflowAttributes, IsConfidential, ComputeBinaryHash, ConfidentialModule.Execute (success, error paths, namespace defaulting, request field forwarding)
- Add handler-level tests verifying confidential routing: confidential attributes bypass engine factory, non-confidential uses it, malformed attributes return error - Add comment documenting "main" as VaultDON default namespace
Replace the local ../chainlink-common replace directive with a proper module reference to commit 177ddc60abbe on the tejaswi/confidential-workflows-codegen branch. Also fix Namespace field assignment (string, not *string) to match the published proto.
The tryConfidentialEngineCreate path was missing the WorkflowID collision diagnostic that tryEngineCreate has. Also remove the #wip changeset tag before merge.
Will add back when confidential workflows is ready for release.
Fix SecretIdentifier.Namespace to use pointer (proto optional field).
…lows - DB migration 0291: add attributes bytea column to workflow_specs_v2 - WorkflowSpec: add Attributes field, persist through ORM - Handler: store payload.Attributes, route confidential workflows to dedicated engine creation path (tryConfidentialEngineCreate) - ConfidentialModule: host.ModuleV2 impl that delegates execution to the confidential-workflows@1.0.0-alpha capability via CapabilitiesRegistry - Plugin registration for confidential-workflows in plugins.private.yaml
The tryConfidentialEngineCreate path was missing the WorkflowID collision diagnostic that tryEngineCreate has. Also remove the #wip changeset tag before merge.
Instantiate the confidential-compute relay handler as a CRE subservice. The handler validates Nitro attestation and dispatches secrets.get / capability.execute messages to VaultDON and capability DONs via the gateway connector. Gated by CL_CONFIDENTIAL_RELAY_TRUSTED_PCRS env var (JSON with PCR0-2 hex values). When unset, no relay service is started.
Gateway-side handler that receives JSON-RPC requests from the enclave, fans them out to relay DON nodes, and aggregates 2F+1 quorum responses. Follows the vault handler pattern but simplified: no authorization, no caching, no OCR3 signatures, no owner-prefixed request IDs.
Add gateway handler type, capability flag, and Feature implementation so the CRE test framework can spin up a relay DON for remote-mode E2E tests.
Pass CL_CONFIDENTIAL_RELAY_CA_ROOTS_PEM env var to relay handler for custom attestation certificate validation. Update go.mod to use remote confidential-compute module commits.
… gateway service name Dockerfile: mount GIT_AUTH_TOKEN during go mod download for private module access in Docker builds. gateway_job.go: set ServiceName "confidential" on the confidential-relay handler so the gateway can route requests from the enclave's RemoteDispatcher (which sends methods like "confidential.capability.execute").
Adds mock binary to the GetCapabilityIDFromCommand and inverse mapping so the standard capabilities delegate properly associates the mock binary with its capability ID.
Picks up fix for relay handler response serialization.
Picks up backward-compat fix that extracts sdkpb.CapabilityRequest Payload and sets Inputs for old-style capabilities.
# Conflicts: # go.mod # go.sum
Points both confidential-http and confidential-workflows at 8c37d30 which has no replace directives, so loopinstall can build them from the Go module cache on a clean checkout.
Previous commit pointed at go.mod without replace directives but with old dep versions. New commit (ed10df3) also bumps the root and framework deps so loopinstall builds compile.
Update relay and attestation pseudo-versions to the latest tejaswi/cw-e2e-test commit which fixes stale attestation pseudo-version and E2E subtest isolation bugs.
The full commit hash was wrong (first 12 chars matched, rest was garbage). Corrected to actual 1efd81acd9949fc79d14a46b1b55faa74fcb436e.
Plumb workflow attributes through RegisterWithContract so confidential workflows can set confidential=true and vault_don_secrets on-chain. Add HTTP URL support to file fetcher (extracts filename from URL path) for workflows where the on-chain URL is HTTP but syncer reads locally. Pass FeatureFlags to confidential engine creation.
product-security-plaid-production
bot
requested review from
DimitriosNaikopoulos,
erikburt,
ettec and
jmank88
Contributor
There was a problem hiding this comment.
Pull request overview
Risk Rating: HIGH (DB migration + workflow execution routing + new gateway handler/subservice wiring)
This PR adds support for workflow attributes end-to-end so confidential workflows can be flagged on-chain and routed to a confidential execution path, and updates the workflow artifact file fetcher to tolerate HTTP(S) URLs when reading from local disk in the syncer.
Changes:
- Plumbs workflow
attributesthrough workflow registration, persistence (workflow_specs_v2.attributes), and syncer engine creation (confidential vs non-confidential routing). - Adds a confidential workflow execution module (
ConfidentialModule) and routes confidential workflows through it. - Wires a new confidential relay gateway handler/subservice and updates build/deps for related components.
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| system-tests/tests/test-helpers/t_helpers.go | Adds workflow registration Attributes and a helper for deploying confidential workflows in system tests. |
| system-tests/lib/cre/workflow/workflow.go | Plumbs attributes into v2 workflow registry contract registration. |
| system-tests/lib/cre/types.go | Adds a new capability flag for confidential relay. |
| system-tests/lib/cre/features/confidential_relay/confidential_relay.go | New system-test feature for confidential relay env/config wiring. |
| plugins/plugins.private.yaml | Adds confidential-workflows plugin + updates refs. |
| deployment/cre/jobs/pkg/gateway_job.go | Adds a new gateway handler type/config for confidential relay. |
| core/store/migrate/migrations/0292_add_workflow_attributes_column.sql | Adds attributes column to workflow_specs_v2. |
| core/services/job/models.go | Adds Attributes []byte to WorkflowSpec. |
| core/services/workflows/artifacts/v2/orm.go | Upserts/updates attributes alongside other workflow spec fields. |
| core/services/workflows/v2/confidential_module.go / *_test.go | Implements and tests confidential execution module + attribute parsing helpers. |
| core/services/workflows/syncer/v2/handler.go / handler_test.go | Persists attributes and routes to confidential engine creation based on them. |
| core/services/workflows/syncer/v2/fetcher.go | File fetcher accepts HTTP(S) URLs by mapping filename to local base path. |
| core/services/workflows/syncer/fetcher.go | Same as above for non-v2 syncer. |
| core/services/gateway/handlers/confidentialrelay/* | Adds confidential relay gateway handler + aggregator + tests. |
| core/services/gateway/handler_factory.go | Wires confidential relay handler into the factory. |
| core/services/cre/cre.go | Starts confidential relay subservice based on env vars. |
| core/capabilities/launcher.go / launcher_test.go | Fixes capability serving in single-DON topologies + adds regression test. |
| core/capabilities/confidentialrelay/service.go | Lifecycle wrapper for confidential relay handler. |
| core/scripts/cre/environment/environment/workflow.go | Updates RegisterWithContract call signature (new attributes arg). |
| core/chainlink.Dockerfile | Adds secret-backed git auth config for go mod download in Docker builds. |
| core/services/standardcapabilities/conversions/conversions.go | Adds “mock” command/capability ID conversion. |
| .changeset/*.md | Release notes entries for the above changes. |
Areas requiring scrupulous human review:
core/services/workflows/syncer/v2/handler.goconfidential routing +tryConfidentialEngineCreatelifecycle/initialization semantics (startup ordering, failure behavior, and parity with the non-confidential path).core/store/migrate/migrations/0292_add_workflow_attributes_column.sql+core/services/workflows/artifacts/v2/orm.go(DB default/null semantics and forward/backward compatibility).core/services/gateway/handlers/confidentialrelay/handler.goconcurrency and request lifecycle correctness (timeout cleanup vs node responses).core/chainlink.Dockerfilesecret-mount approach for module download (CI/CD compatibility and token handling).
Reviewer recommendations (from CODEOWNERS):
/core/services/workflows/**,/deployment/cre/**:@smartcontractkit/keystone/core/capabilities/**: @smartcontractkit/keystone,@smartcontractkit/capabilities-team/core/chainlink.Dockerfile:@smartcontractkit/devex-cicd(plus @smartcontractkit/core/@smartcontractkit/foundations per CODEOWNERS)
|
|
||
| for _, er := range expiredRequests { | ||
| var nodeResponses string | ||
| for nodeKey, nodeResponse := range er.responses { |
There was a problem hiding this comment.
er.responses is a map that is also written to by HandleNodeMessage via addResponseForNode. Iterating it here without taking er.mu can cause a data race / panic (“concurrent map iteration and map write”). Consider using er.copiedResponses() (or locking er.mu) before iterating/formatting responses.
Suggested change
| for nodeKey, nodeResponse := range er.responses { | |
| responses := er.copiedResponses() | |
| for nodeKey, nodeResponse := range responses { |
| l.Debugw("aggregating responses, waiting for other nodes...", "error", err) | ||
| return nil | ||
| case err != nil: | ||
| l.Error("quorum unobtainable, returning response to user...", "error", err, "responses", maps.Values(ar.responses)) |
There was a problem hiding this comment.
This log line reads ar.responses without holding ar.mu. Since responses is a map mutated concurrently, this can race/panic. Use ar.copiedResponses() (or lock around reading ar.responses) when logging/inspecting responses.
Suggested change
| l.Error("quorum unobtainable, returning response to user...", "error", err, "responses", maps.Values(ar.responses)) | |
| l.Error("quorum unobtainable, returning response to user...", "error", err, "responses", maps.Values(copiedResponses)) |
|
|
||
| confidential, err := v2.IsConfidential(spec.Attributes) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to parse workflow attributes: %w", err) |
There was a problem hiding this comment.
v2.IsConfidential already returns errors wrapped with the prefix "failed to parse workflow attributes" (via ParseWorkflowAttributes). Wrapping again with the same message here will duplicate the prefix in the final error string. Consider either returning err directly or wrapping with a higher-level message (e.g. include workflow ID/source) instead.
Suggested change
| return fmt.Errorf("failed to parse workflow attributes: %w", err) | |
| return fmt.Errorf("failed to determine workflow confidentiality: %w", err) |
| ) error { | ||
| attrs, err := v2.ParseWorkflowAttributes(spec.Attributes) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to parse workflow attributes: %w", err) |
There was a problem hiding this comment.
v2.ParseWorkflowAttributes already wraps JSON errors with "failed to parse workflow attributes". Wrapping it again with the same prefix here will duplicate the message. Consider returning err directly or wrapping with more contextual info (e.g. workflowID) instead of repeating the same text.
Suggested change
| return fmt.Errorf("failed to parse workflow attributes: %w", err) | |
| return fmt.Errorf("workflowID %s: %w", spec.WorkflowID, err) |
Comment on lines
+215
to
+227
| var fullPath string | ||
| 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. | ||
| fullPath = filepath.Join(basePath, filepath.Base(u.Path)) | ||
| } else { | ||
| fullPath = filepath.Clean(u.Path) | ||
| // ensure that the incoming request URL is either relative or absolute but within the basePath | ||
| if !filepath.IsAbs(fullPath) { | ||
| // If it's not absolute, we assume it's relative to the basePath | ||
| fullPath = filepath.Join(basePath, fullPath) | ||
| } |
There was a problem hiding this comment.
This change adds a new behavior path (when the file fetcher receives an HTTP(S) URL, it resolves filepath.Base(u.Path) against basePath). fetcher_test.go currently covers relative paths, absolute paths, and traversal attempts, but doesn’t cover this new HTTP(S)-URL-as-filename behavior. Please add a unit test that passes an http://.../somefile URL to the file fetcher and asserts it reads basePath/somefile (and that query strings / path segments don’t bypass the basePath constraint).
| fullPath = filepath.Join(basePath, fullPath) | ||
| } | ||
| } | ||
| if !strings.HasPrefix(fullPath, basePath) { |
There was a problem hiding this comment.
This change introduces a new code path where the file fetcher accepts an HTTP(S) URL and maps it to a local filename (filepath.Base(u.Path) under basePath). The existing unit tests for the file fetcher don’t cover this case. Please add a test that provides an HTTP(S) URL and verifies it reads the expected local file under basePath (and still rejects traversal / out-of-base paths).
Suggested change
| if !strings.HasPrefix(fullPath, basePath) { | |
| // Normalize the full path and ensure it is within basePath to prevent directory traversal. | |
| fullPath = filepath.Clean(fullPath) | |
| rel, err := filepath.Rel(basePath, fullPath) | |
| if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { |
Comment on lines
+38
to
+44
| members := make([]config.NodeConfig, numNodes) | ||
| for i := range numNodes { | ||
| members[i] = config.NodeConfig{ | ||
| Name: fmt.Sprintf("node%d", i), | ||
| Address: fmt.Sprintf("0x%04d", i), | ||
| } | ||
| } |
There was a problem hiding this comment.
This loop won’t compile: range can’t be used on an int (numNodes). Use an index loop (for i := 0; i < numNodes; i++ { ... }) or range over the members slice instead.
| require.NoError(t, err) | ||
|
|
||
| // Send 3 matching responses (2F+1 = 3) | ||
| for i := range 3 { |
There was a problem hiding this comment.
This loop won’t compile: range 3 is invalid because 3 is an int, not an iterable. Use for i := 0; i < 3; i++ { ... }.
Suggested change
| for i := range 3 { | |
| for i := 0; i < 3; i++ { |
nadahalli
changed the title
Contributor
Author
|
Recreating from a clean branch stacked on #21298 (removes merged commits from other PRs) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three changes needed for the confidential workflows engine-path E2E test (confidential-compute#260). The test validates the full flow: syncer detects
confidential=trueworkflow, createsConfidentialModule, cron trigger fires, module delegates to theconfidential-workflowscapability, enclave runs WASM, mock capability on relay DON receives the request.Changes
1. Workflow attributes plumbing
RegisterWithContractandregisterWorkflowV2now accept anattributes []byteparameter, passed through toregistry.UpsertWorkflow. Confidential workflows set{"confidential":true,"vault_don_secrets":[{"key":"MOCK_SECRET"}]}so the syncer routes them toConfidentialModule(see #21298).All existing callers pass
nil(no behavior change).CompileAndDeployConfidentialWorkflowhelper added tot_helpers.gofor E2E tests that need attributes.Files:
system-tests/lib/cre/workflow/workflow.go,system-tests/tests/test-helpers/t_helpers.go,core/scripts/cre/environment/environment/workflow.go2. File fetcher HTTP URL support
Confidential workflows have a dual-use binary URL: the on-chain URL must be HTTP (so the enclave can fetch the binary), but the syncer's file-based fetcher reads from the local container filesystem. The file fetcher now detects HTTP(S) URLs and extracts the filename via
filepath.Base(u.Path), resolving it against the configured base path.Without this, the file fetcher rejects the HTTP URL with "request URL is not within the basePath".
Files:
core/services/workflows/syncer/fetcher.go,core/services/workflows/syncer/v2/fetcher.go3. FeatureFlags in confidential engine creation
tryConfidentialEngineCreateinhandler.gowas missingFeatureFlags: h.featureFlagsin the engine config, causing a nil pointer panic when the engine checked feature flags during trigger registration.File:
core/services/workflows/syncer/v2/handler.goRelated PRs