Stricter validations for centralized registry workflows by prashantkumar1982 · Pull Request #23013 · smartcontractkit/chainlink

prashantkumar1982 · 2026-07-01T22:26:03Z

https://smartcontract-it.atlassian.net/browse/PRIV-588

github-actions · 2026-07-01T22:27:13Z

I see you updated files related to core. Please run make gocs in the root directory to add a changeset as well as in the text include at least one of the following tags:

#added For any new functionality added.
#breaking_change For any functionality that requires manual action for the node to boot.
#bugfix For bug fixes.
#changed For any change to the existing functionality.
#db_update For any feature that introduces updates to database schema.
#deprecation_notice For any upcoming deprecation functionality.
#internal For changesets that need to be excluded from the final changelog.
#nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
#removed For any functionality/config that is removed.
#updated For any functionality that is updated.
#wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

github-actions · 2026-07-01T22:27:47Z

✅ No conflicts with other open PRs targeting develop

trunk-io · 2026-07-01T22:49:40Z

Failed Test	Failure Summary	Logs
`Test_CCIP_TokenTransfer_BnM_Aptos2EVM`	The test failed because the Aptos node API could not be accessed on port 8080, which is the required port for exposing the API.	Logs ↗︎
`Test_CCIP_TokenTransfer_BnM_Aptos2EVM`	The test failed because the Aptos node API could not be exposed on port 8080, causing a connection issue.	Logs ↗︎
`Test_CCIP_RegulatedTokenTransfer_Aptos2EVM`	The test failed because the Aptos node API could not be accessed on port 8080, which is the required port for exposing the API.	Logs ↗︎
`Test_CCIP_AptosMessageHasher_FuzzGeneratedCases`	The test failed due to a port conflict because the Aptos node API can only be exposed on port 8080, but the test attempted to use different ports.	Logs ↗︎

... and 16 more

View Full Report ↗︎ ⋅ Docs

ibrajer · 2026-07-03T06:52:53Z

+// defaultTenantID is the fallback tenant numeric id for the CRE environment when none
+// is configured on the job spec. It matches defaultJWTAuthJobSpecTenantID in
+// core/capabilities/vault and cre-platform-graphql's account service.
+const defaultTenantID uint64 = 1


I think we should be very careful when using defaults - if the tenant ID ever disappears from the job spec by mistake, DONs will start rejecting requests. I would rather enforce it in the job spec if that's a possibility.

We're using the same pattern in other vault checks too, to default to a tenantID of 1 if not provided.
My thinking is if you fail to set it, things will break, and you then must set it correctly in the right job-specs.

I could also make this an explicit failure if tenantID wasn't set. But that can only be done after we first fix all job-specs and deploy them across all environments. Otherwise this change will start breaking rollouts immediately.

But if things break, isn't it harder for you to fix an invalid tenant ID because you have to go through the process of job spec rollout for the entire DON and the resolution time depends on how quickly a NOP can accept the updated job spec?

I understand that you can't get rid of defaults immediately due to what you have described above, but just asking to consider the risks one more time (I'm unfamiliar with this codebase, so I can't tell you if this is a bad or a good idea, but I'll defer to @cedric-cordenier to weigh in).

My idea was to set the right tenantID on all environments job-specs, and push them out.
Once set correctly, there's no reason to believe somebody will incorrectly reset them.
Here was the PR for it: https://github.com/smartcontractkit/chainlink-deployments/pull/16126

ibrajer · 2026-07-03T06:55:10Z

+// claim without on-chain proof and therefore require independent owner<->orgID
+// verification before the workflow is admitted.
+func isCentralizedWorkflowSource(source string) bool {
+	return strings.HasPrefix(source, "grpc:") || strings.HasPrefix(source, "file:")


Where does this source string come from?

Here:

chainlink/core/services/workflows/syncer/v2/types.go

Line 146 in 61bdf87

Source string // source that provided this workflow metadata

And that field is set here:

chainlink/core/services/workflows/syncer/v2/grpc_workflow_source.go

Line 355 in 6eb01ae

Source: g.SourceIdentifier(),

I will also ask @cedric-cordenier to confirm again, that this is the right authoritative place to read the source from, and isn't spoofable.

Okay, I'm asking because I'm pretty sure that the workflow registry does not provide this field, so the workflow metadata must have been augmented during processing on the node side.

ibrajer · 2026-07-03T06:57:22Z

+	if orgID == "" {
+		// Without an orgID we cannot derive the expected owner; the resolution failure
+		// was already logged by fetchOrganizationID.
+		h.lggr.Warnw("skipping centralized workflow owner/orgID verification: no organization ID resolved",


Are we sure we don't want to treat this as an error instead? What happens when you can't resolve an org, do you usually just proceed?

Yes, in my understanding too, if we cannot fetch orgID, we should likely fail even running any workflow, not just this specific check.
Because we have other sub-components depending on the orgID to be set correctly.
But not sure why the code currently is set to only warn on missing orgID.

I didn't explicitly force that for this check, because it means all workflows will start failing without orgID, which currently isn't so.

@cedric-cordenier do you know if there's a harm in enforcing strict orgID presence?

Atm we fail open, so I wouldn't start enforcing this as part of this PR
I think we /should/ aim to enforce it long-term though, but in my mind this would be part of the "reliable org resolver" work we talked about the other day (with the static mapping)

ibrajer · 2026-07-03T06:58:55Z

+	}
+	derivedHex := hex.EncodeToString(derived)
+
+	if !strings.EqualFold(strings.TrimPrefix(strings.TrimSpace(ownerHex), "0x"), derivedHex) {


Recommended to use ToLower as well, in case one of the addresses turns out in checksum format, and the other one doesn't.

github-actions · 2026-07-06T17:25:59Z

CORA - Pending Reviewers

All codeowners have approved! ✅

For more details, see the full review summary.

cedric-cordenier · 2026-07-06T17:29:27Z

 	configJSON := spec.StandardCapabilitiesSpec.Config
+	if d.workflowRegistrySyncer != nil {
+		if tenantID, ok := parseTenantID(configJSON); ok {
+			d.workflowRegistrySyncer.SetTenantID(tenantID)


:( This feels pretty wrong tbh -- what happens if multiple capabilities have different tenant IDs? Atm the last one ones and we're probably not guaranteed that it will consistently win

cedric-cordenier · 2026-07-06T17:33:20Z

+// so the caller rejects the workflow (fail-closed). If the orgID cannot be resolved
+// or the derivation fails, verification is skipped (returns nil) rather than blocking
+// legitimate workflows on a transient Linking Service outage.
+func (h *eventHandler) verifyCentralizedOwnerOrgMapping(source, ownerHex, orgID string) error {


Did you consider putting this logic in the GRPC contract source instead? Feels like that is the more natural place for it -- we should check that we can re-derive the owner from the org ID + tenant ID there

cl-sonarqube-production · 2026-07-06T17:36:29Z

Quality Gate passed

Issues
2 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

prashantkumar1982 added 3 commits July 1, 2026 15:24

Stricter validations for centralized registry workflows

4d13864

add missed files

3b0569e

merge develop

fe96a9b

prashantkumar1982 added 2 commits July 1, 2026 15:31

fix tests

abed036

lint fixes

b292083

prashantkumar1982 added 2 commits July 2, 2026 15:42

nits

a4305dc

merge develop to branch

66966b7

prashantkumar1982 marked this pull request as ready for review July 2, 2026 23:24

prashantkumar1982 requested review from a team as code owners July 2, 2026 23:24

product-security-plaid-production Bot requested review from jmank88 and justinkaseman July 2, 2026 23:24

prashantkumar1982 requested review from cedric-cordenier, gneven and ibrajer July 2, 2026 23:25

ibrajer reviewed Jul 3, 2026

View reviewed changes

prashantkumar1982 mentioned this pull request Jul 3, 2026

Add CentralizedWorkflowOwnerVerificationEnabled CRE gate setting smartcontractkit/chainlink-common#2221

Merged

prashantkumar1982 added 2 commits July 3, 2026 14:02

add gate for new code changes

10c455e

lint

636815b

prashantkumar1982 requested a review from a team as a code owner July 3, 2026 21:04

product-security-plaid-production Bot requested a review from ChrisAmora July 3, 2026 21:04

prashantkumar1982 added 2 commits July 3, 2026 14:09

go mod fix

efccd22

merge develop into branch

dc67784

prashantkumar1982 added 2 commits July 6, 2026 10:18

go mod fixes

3676919

make generate

6a85c95

prashantkumar1982 requested a review from a team as a code owner July 6, 2026 17:25

cedric-cordenier reviewed Jul 6, 2026

View reviewed changes

jmank88 approved these changes Jul 6, 2026

View reviewed changes

Uh oh!

Conversation

prashantkumar1982 commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 1, 2026

Uh oh!

github-actions Bot commented Jul 1, 2026

Uh oh!

trunk-io Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CORA - Pending Reviewers

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cl-sonarqube-production Bot commented Jul 6, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

prashantkumar1982 commented Jul 1, 2026 •

edited

Loading

trunk-io Bot commented Jul 1, 2026 •

edited

Loading

github-actions Bot commented Jul 6, 2026 •

edited

Loading