From 97dc81b9248214e6c1ec409fa25c94f771842a12 Mon Sep 17 00:00:00 2001 From: Kenrick Tan Date: Mon, 7 Sep 2026 16:10:12 +0800 Subject: [PATCH 01/12] BCE-11514: fix the AWS decryption key encoding and add cluster selection Two changes for pointing web3signer at the key operation service's keystore table. The provider: aws branch piped the KMS plaintext through base64 -d before writing DECRYPTION_KEY. sync-keys base64-decodes that value itself, and the Azure branch writes base64, so the AWS branch was handing it raw bytes. Worse, raw key material does not survive a shell command substitution: a key containing a null byte was silently truncated and sync-keys then failed with "Incorrect AES key length (0 bytes)". A random 32-byte key contains a null byte about 11.8 percent of the time, so the failure came and went between key generations. The value now stays base64, matching the Azure branch. Adds dbKeystoreClientClusterId and dbKeystoreAllClusters, which render --client-cluster-id and --all-clusters (sync-keys 0.4.2+). A keystore table carrying a client_cluster_id column holds more than one cluster's keys, and without a predicate every web3signer sharing that database loads every cluster's private keys. Setting both is a template error rather than a runtime surprise. Renders verified: the pilot values produce --table-name and --client-cluster-id; the all-clusters variant produces --all-clusters; setting both fails the template; and a release using neither renders byte-identical to origin/main apart from the chart version label. --- charts/web3signer/Chart.yaml | 2 +- charts/web3signer/templates/_helpers.tpl | 17 ++++++++++++++++- charts/web3signer/values.yaml | 10 ++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/charts/web3signer/Chart.yaml b/charts/web3signer/Chart.yaml index a6e35f8..412bd4a 100644 --- a/charts/web3signer/Chart.yaml +++ b/charts/web3signer/Chart.yaml @@ -15,7 +15,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 6.4.4 +version: 6.5.0 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to diff --git a/charts/web3signer/templates/_helpers.tpl b/charts/web3signer/templates/_helpers.tpl index e7087dd..f04dd06 100644 --- a/charts/web3signer/templates/_helpers.tpl +++ b/charts/web3signer/templates/_helpers.tpl @@ -88,11 +88,16 @@ Create the name of the service account to use export AWS_SESSION_TOKEN=$(echo $CREDS | sed -n 's/.*"SessionToken": "\([^"]*\)".*/\1/p') {{- end }} echo "$ENCRYPTED_DECRYPTION_KEY" | base64 -d > /tmp/ciphertext.bin + # Leave the value base64-encoded. sync-keys base64-decodes DECRYPTION_KEY itself, and + # the Azure branch below likewise writes base64. Decoding here also lost bytes: raw key + # material does not survive a shell command substitution, so a key containing a null + # byte was silently truncated and sync-keys then failed with + # "Incorrect AES key length". That affects roughly one random 32-byte key in eight. DECRYPTED=$(aws kms decrypt \ --region "$AWS_REGION" \ --ciphertext-blob fileb:///tmp/ciphertext.bin \ --output text \ - --query Plaintext | base64 -d) + --query Plaintext) rm -f /tmp/ciphertext.bin {{- else if eq $provider "azure" }} echo "Decrypting secret using Azure Key Vault..." @@ -184,6 +189,16 @@ Create the name of the service account to use {{- $renderedArgs = append $renderedArgs "--table-name" }} {{- $renderedArgs = append $renderedArgs .root.Values.dbKeystoreTableName }} {{- end }} + {{- if and .root.Values.dbKeystoreClientClusterId .root.Values.dbKeystoreAllClusters }} + {{- fail "web3signer: set dbKeystoreClientClusterId or dbKeystoreAllClusters, not both" }} + {{- end }} + {{- if and $isFetchKeys .root.Values.dbKeystoreClientClusterId }} + {{- $renderedArgs = append $renderedArgs "--client-cluster-id" }} + {{- $renderedArgs = append $renderedArgs .root.Values.dbKeystoreClientClusterId }} + {{- end }} + {{- if and $isFetchKeys .root.Values.dbKeystoreAllClusters }} + {{- $renderedArgs = append $renderedArgs "--all-clusters" }} + {{- end }} {{- if and $isFetchKeys $useKeyVaultSecrets (not .root.Values.dbKeystoreUrl) }} {{- $renderedArgs = append $renderedArgs "--db-url" }} {{- $renderedArgs = append $renderedArgs "$DB_KEYSTORE_URL" }} diff --git a/charts/web3signer/values.yaml b/charts/web3signer/values.yaml index 25c226f..129a91d 100644 --- a/charts/web3signer/values.yaml +++ b/charts/web3signer/values.yaml @@ -25,6 +25,16 @@ dbKeystoreUrl: "" ## dbKeystoreTableName: "" +## Restrict fetched keys to a single cluster (sync-keys 0.4.2+). Required when the keystore +## table carries a client_cluster_id column, because that table holds more than one cluster's +## keys and sync-keys otherwise refuses to run. Leave empty for the agent-managed table, +## which has no such column. +dbKeystoreClientClusterId: "" +## Deliberately load every cluster's keys from a cluster-scoped table (sync-keys 0.4.2+). +## Only correct when this signer really does serve every cluster in that table. Mutually +## exclusive with dbKeystoreClientClusterId. +dbKeystoreAllClusters: false + ## The key for decrypting private keys is generated with stakewise-cli sync-db command ## Use this for plain text decryption key (legacy mode) ## From 54197444e0730419a018e343943f8a1fa91aa1f4 Mon Sep 17 00:00:00 2001 From: Kenrick Tan Date: Mon, 7 Sep 2026 16:58:23 +0800 Subject: [PATCH 02/12] BCE-11514: note that the new options need a sync-keys tag override The fetch-keys init container defaults to sync-keys 0.4.1, which does not accept --client-cluster-id or --all-clusters. Releases inherit that default rather than pinning their own tag -- all six production signers do -- so bumping it here would change the image every web3signer pulls. Left at 0.4.1, with the requirement to override recorded next to the options that need it. --- charts/web3signer/values.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/charts/web3signer/values.yaml b/charts/web3signer/values.yaml index 129a91d..eab111e 100644 --- a/charts/web3signer/values.yaml +++ b/charts/web3signer/values.yaml @@ -25,6 +25,12 @@ dbKeystoreUrl: "" ## dbKeystoreTableName: "" +## NOTE: the fetch-keys init container below still defaults to sync-keys 0.4.1, which does not +## understand the two options here. A release that sets either one must also pin +## initContainers[fetch-keys].image.tag to 0.4.2 or newer. The default is deliberately left +## alone: releases inherit it, so bumping it here would change the image every existing +## web3signer pulls, production included. +## ## Restrict fetched keys to a single cluster (sync-keys 0.4.2+). Required when the keystore ## table carries a client_cluster_id column, because that table holds more than one cluster's ## keys and sync-keys otherwise refuses to run. Leave empty for the agent-managed table, From 36a7ea249f42b4e8827e3b0f6823788588021c52 Mon Sep 17 00:00:00 2001 From: Kenrick Tan Date: Mon, 7 Sep 2026 16:59:36 +0800 Subject: [PATCH 03/12] BCE-11514: correct the note on the sync-keys default tag The previous wording claimed bumping the default would change the image every web3signer pulls including production. That is wrong: every release pins an exact chart version, and all six bfceu production signers are on 6.3.1, so publishing a new chart version reaches nobody until a HelmRelease is edited. The real reason to leave the default alone is narrower -- it would bundle a sync-keys upgrade into whichever release next adopts this chart version, which should be a separate decision. Releases using the new options pin the tag themselves. --- charts/web3signer/values.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/charts/web3signer/values.yaml b/charts/web3signer/values.yaml index eab111e..d8009b9 100644 --- a/charts/web3signer/values.yaml +++ b/charts/web3signer/values.yaml @@ -28,8 +28,9 @@ dbKeystoreTableName: "" ## NOTE: the fetch-keys init container below still defaults to sync-keys 0.4.1, which does not ## understand the two options here. A release that sets either one must also pin ## initContainers[fetch-keys].image.tag to 0.4.2 or newer. The default is deliberately left -## alone: releases inherit it, so bumping it here would change the image every existing -## web3signer pulls, production included. +## alone: every release pins a chart version, so changing it would not affect anyone +## immediately, but it would silently bundle a sync-keys upgrade into whichever release next +## adopts this chart version. Image bumps should be their own decision. ## ## Restrict fetched keys to a single cluster (sync-keys 0.4.2+). Required when the keystore ## table carries a client_cluster_id column, because that table holds more than one cluster's From e548b38e1dacc28ac7ef94bcb61abdb96b087a76 Mon Sep 17 00:00:00 2001 From: Kenrick Tan Date: Mon, 7 Sep 2026 18:04:47 +0800 Subject: [PATCH 04/12] BCE-11514: take a list of cluster ids rather than one A signer can legitimately serve several clusters -- QA's use1/hoodi-1 serves two -- and a single value forced those releases to fall back to dbKeystoreAllClusters, which then silently picks up any cluster added to the same database later. dbKeystoreClientClusterId becomes dbKeystoreClientClusterIds, a list, and the template repeats --client-cluster-id once per entry. An empty entry fails the template rather than reaching sync-keys, where a repeatable option makes an empty value indistinguishable from "a cluster was named". Naming the clusters is now preferable to dbKeystoreAllClusters in every case, because the set is closed. Renders verified: two ids produce the flag twice, one id once, an empty entry fails, and a release setting neither option still renders identically to origin/main. helm lint passes. --- charts/web3signer/templates/_helpers.tpl | 13 +++++++++---- charts/web3signer/values.yaml | 20 ++++++++++++-------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/charts/web3signer/templates/_helpers.tpl b/charts/web3signer/templates/_helpers.tpl index f04dd06..0098e98 100644 --- a/charts/web3signer/templates/_helpers.tpl +++ b/charts/web3signer/templates/_helpers.tpl @@ -189,12 +189,17 @@ Create the name of the service account to use {{- $renderedArgs = append $renderedArgs "--table-name" }} {{- $renderedArgs = append $renderedArgs .root.Values.dbKeystoreTableName }} {{- end }} - {{- if and .root.Values.dbKeystoreClientClusterId .root.Values.dbKeystoreAllClusters }} - {{- fail "web3signer: set dbKeystoreClientClusterId or dbKeystoreAllClusters, not both" }} + {{- if and .root.Values.dbKeystoreClientClusterIds .root.Values.dbKeystoreAllClusters }} + {{- fail "web3signer: set dbKeystoreClientClusterIds or dbKeystoreAllClusters, not both" }} + {{- end }} + {{- if $isFetchKeys }} + {{- range .root.Values.dbKeystoreClientClusterIds }} + {{- if not . }} + {{- fail "web3signer: dbKeystoreClientClusterIds must not contain an empty value" }} {{- end }} - {{- if and $isFetchKeys .root.Values.dbKeystoreClientClusterId }} {{- $renderedArgs = append $renderedArgs "--client-cluster-id" }} - {{- $renderedArgs = append $renderedArgs .root.Values.dbKeystoreClientClusterId }} + {{- $renderedArgs = append $renderedArgs . }} + {{- end }} {{- end }} {{- if and $isFetchKeys .root.Values.dbKeystoreAllClusters }} {{- $renderedArgs = append $renderedArgs "--all-clusters" }} diff --git a/charts/web3signer/values.yaml b/charts/web3signer/values.yaml index d8009b9..c873a24 100644 --- a/charts/web3signer/values.yaml +++ b/charts/web3signer/values.yaml @@ -32,14 +32,18 @@ dbKeystoreTableName: "" ## immediately, but it would silently bundle a sync-keys upgrade into whichever release next ## adopts this chart version. Image bumps should be their own decision. ## -## Restrict fetched keys to a single cluster (sync-keys 0.4.2+). Required when the keystore -## table carries a client_cluster_id column, because that table holds more than one cluster's -## keys and sync-keys otherwise refuses to run. Leave empty for the agent-managed table, -## which has no such column. -dbKeystoreClientClusterId: "" -## Deliberately load every cluster's keys from a cluster-scoped table (sync-keys 0.4.2+). -## Only correct when this signer really does serve every cluster in that table. Mutually -## exclusive with dbKeystoreClientClusterId. +## Restrict fetched keys to these clusters. Required when the keystore table carries a +## client_cluster_id column, because that table holds more than one cluster's keys and +## sync-keys otherwise refuses to run. Leave empty for the agent-managed table, which has no +## such column. +## +## A list, because one signer can legitimately serve several clusters. Naming them is +## preferable to dbKeystoreAllClusters: the set is closed, so a cluster added to the same +## database later is not served unless it is added here. +dbKeystoreClientClusterIds: [] +## Deliberately load every cluster's keys from a cluster-scoped table. Only correct when this +## signer really does serve every cluster in that table, including ones added later. Prefer +## naming the clusters. Mutually exclusive with dbKeystoreClientClusterIds. dbKeystoreAllClusters: false ## The key for decrypting private keys is generated with stakewise-cli sync-db command From 058aa3010601fe4bf8735cb73c4b9716a4c441a0 Mon Sep 17 00:00:00 2001 From: kenrick-g Date: Mon, 7 Sep 2026 18:35:03 +0800 Subject: [PATCH 05/12] fix: render the sync-keys cluster flags on both arg paths The cluster flags were only appended inside the encryptedDecryptionKey branch, which execs sync-keys through a shell. The plain command/args branch appended only --table-name, so on any release not using the encrypted secret the dbKeystoreClientClusterIds and dbKeystoreAllClusters values were accepted and silently ignored. Every live QA web3signer uses that plain branch, so the scoping feature would have been inert on exactly the releases being migrated: the values would look right in the HelmRelease while sync-keys ran unscoped. The flags are now built once into $fetchKeysFlags before either branch and consumed by both, so a flag cannot be added to one path and missed on the other. Also adds dbKeystoreAllowNoKeys, matching sync-keys' --allow-no-keys, for a signer that is deployed but serves no validators. QA's euw1/hoodi-2 is one: zero rows in its keystore table with web3signer running 3/3 ready. Verified by render: a legacy release is byte-identical to origin/main apart from the chart version; both branches now emit --client-cluster-id once per entry; the mutual-exclusion and empty-value guards still fail the template. --- charts/web3signer/templates/_helpers.tpl | 53 ++++++++++++++---------- charts/web3signer/values.yaml | 5 +++ 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/charts/web3signer/templates/_helpers.tpl b/charts/web3signer/templates/_helpers.tpl index 0098e98..4b0d70d 100644 --- a/charts/web3signer/templates/_helpers.tpl +++ b/charts/web3signer/templates/_helpers.tpl @@ -167,6 +167,33 @@ Create the name of the service account to use {{- $useKeyVaultSecrets := and $useEncryptedSecret .root.Values.encryptedDecryptionKey.useKeyVaultSecrets -}} {{- $isFetchKeys := eq .container.name "fetch-keys" -}} {{- $isMigrations := eq .container.name "migrations" -}} +{{- /* + Extra sync-keys flags for the fetch-keys container. Built once here because there are two + arg-rendering paths below -- the encryptedDecryptionKey one that execs through a shell, and + the plain command/args one -- and a flag added to only one of them is silently dropped on + every release using the other. +*/ -}} +{{- $fetchKeysFlags := list -}} +{{- if $isFetchKeys -}} + {{- if and .root.Values.dbKeystoreClientClusterIds .root.Values.dbKeystoreAllClusters -}} + {{- fail "web3signer: set dbKeystoreClientClusterIds or dbKeystoreAllClusters, not both" -}} + {{- end -}} + {{- if .root.Values.dbKeystoreTableName -}} + {{- $fetchKeysFlags = concat $fetchKeysFlags (list "--table-name" .root.Values.dbKeystoreTableName) -}} + {{- end -}} + {{- range .root.Values.dbKeystoreClientClusterIds -}} + {{- if not . -}} + {{- fail "web3signer: dbKeystoreClientClusterIds must not contain an empty value" -}} + {{- end -}} + {{- $fetchKeysFlags = concat $fetchKeysFlags (list "--client-cluster-id" .) -}} + {{- end -}} + {{- if .root.Values.dbKeystoreAllClusters -}} + {{- $fetchKeysFlags = concat $fetchKeysFlags (list "--all-clusters") -}} + {{- end -}} + {{- if .root.Values.dbKeystoreAllowNoKeys -}} + {{- $fetchKeysFlags = concat $fetchKeysFlags (list "--allow-no-keys") -}} + {{- end -}} +{{- end -}} - name: {{ .container.name }} {{- if .container.image }} @@ -185,25 +212,7 @@ Create the name of the service account to use {{- range .container.args }} {{- $renderedArgs = append $renderedArgs (tpl . $.root) }} {{- end }} - {{- if and $isFetchKeys .root.Values.dbKeystoreTableName }} - {{- $renderedArgs = append $renderedArgs "--table-name" }} - {{- $renderedArgs = append $renderedArgs .root.Values.dbKeystoreTableName }} - {{- end }} - {{- if and .root.Values.dbKeystoreClientClusterIds .root.Values.dbKeystoreAllClusters }} - {{- fail "web3signer: set dbKeystoreClientClusterIds or dbKeystoreAllClusters, not both" }} - {{- end }} - {{- if $isFetchKeys }} - {{- range .root.Values.dbKeystoreClientClusterIds }} - {{- if not . }} - {{- fail "web3signer: dbKeystoreClientClusterIds must not contain an empty value" }} - {{- end }} - {{- $renderedArgs = append $renderedArgs "--client-cluster-id" }} - {{- $renderedArgs = append $renderedArgs . }} - {{- end }} - {{- end }} - {{- if and $isFetchKeys .root.Values.dbKeystoreAllClusters }} - {{- $renderedArgs = append $renderedArgs "--all-clusters" }} - {{- end }} + {{- $renderedArgs = concat $renderedArgs $fetchKeysFlags }} {{- if and $isFetchKeys $useKeyVaultSecrets (not .root.Values.dbKeystoreUrl) }} {{- $renderedArgs = append $renderedArgs "--db-url" }} {{- $renderedArgs = append $renderedArgs "$DB_KEYSTORE_URL" }} @@ -238,13 +247,13 @@ Create the name of the service account to use command: {{- (tpl (toYaml .container.command) .root) | nindent 4 }} {{- end }} - {{- if or .container.args (and $isFetchKeys .root.Values.dbKeystoreTableName) }} + {{- if or .container.args $fetchKeysFlags }} args: {{- if .container.args }} {{- (tpl (toYaml .container.args) .root) | nindent 2 }} {{- end }} - {{- if and $isFetchKeys .root.Values.dbKeystoreTableName }} - {{- (list "--table-name" .root.Values.dbKeystoreTableName | toYaml) | nindent 2 }} + {{- if $fetchKeysFlags }} + {{- (toYaml $fetchKeysFlags) | nindent 2 }} {{- end }} {{- end }} {{- end }} diff --git a/charts/web3signer/values.yaml b/charts/web3signer/values.yaml index c873a24..7937461 100644 --- a/charts/web3signer/values.yaml +++ b/charts/web3signer/values.yaml @@ -45,6 +45,11 @@ dbKeystoreClientClusterIds: [] ## signer really does serve every cluster in that table, including ones added later. Prefer ## naming the clusters. Mutually exclusive with dbKeystoreClientClusterIds. dbKeystoreAllClusters: false +## Start with an empty keystore when the keystore table returns no rows, instead of failing +## the init container. Set this only for a signer that is deployed but serves no validators +## yet; otherwise a wrong cluster id or an unpopulated table should stop the pod rather than +## bring up a signer that cannot sign. +dbKeystoreAllowNoKeys: false ## The key for decrypting private keys is generated with stakewise-cli sync-db command ## Use this for plain text decryption key (legacy mode) From 5c4ab7983d94fc131ea5c97a3493ef6b1e941bdd Mon Sep 17 00:00:00 2001 From: kenrick-g Date: Mon, 7 Sep 2026 20:13:32 +0800 Subject: [PATCH 06/12] docs: pin the required sync-keys version at 0.5.0 and note the idle-signer flag The note said 0.4.2, but sync-keys releases via conventional commits with default_bump: false, and PR #38 carries feat: commits on top of v0.4.1, so the version that first understands these options is 0.5.0. Also records that when the chart's default image tag moves, a namespace whose keystore query returns no rows needs dbKeystoreAllowNoKeys in the same change. QA's euw1/hoodi-2 is one: zero rows with web3signer running 3/3 ready. --- charts/web3signer/values.yaml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/charts/web3signer/values.yaml b/charts/web3signer/values.yaml index 7937461..b8d66e1 100644 --- a/charts/web3signer/values.yaml +++ b/charts/web3signer/values.yaml @@ -26,12 +26,16 @@ dbKeystoreUrl: "" dbKeystoreTableName: "" ## NOTE: the fetch-keys init container below still defaults to sync-keys 0.4.1, which does not -## understand the two options here. A release that sets either one must also pin -## initContainers[fetch-keys].image.tag to 0.4.2 or newer. The default is deliberately left +## understand the options here. A release that sets any of them must also pin +## initContainers[fetch-keys].image.tag to 0.5.0 or newer. The default is deliberately left ## alone: every release pins a chart version, so changing it would not affect anyone ## immediately, but it would silently bundle a sync-keys upgrade into whichever release next ## adopts this chart version. Image bumps should be their own decision. ## +## When that default does move, a namespace whose keystore query returns no rows needs +## dbKeystoreAllowNoKeys in the same change, or its init container will fail. QA's +## euw1/hoodi-2 is one such namespace. +## ## Restrict fetched keys to these clusters. Required when the keystore table carries a ## client_cluster_id column, because that table holds more than one cluster's keys and ## sync-keys otherwise refuses to run. Leave empty for the agent-managed table, which has no From a978330428cd1f5314bc2cffbc7f66bc3798d6b0 Mon Sep 17 00:00:00 2001 From: kenrick-g Date: Mon, 7 Sep 2026 20:14:49 +0800 Subject: [PATCH 07/12] docs: required sync-keys version is 0.4.2, not 0.5.0 sync-keys #38 was retitled from feat: to fix: so it releases as a patch. With squash merge the tag action reads only the PR title as the commit header, so v0.4.1 becomes v0.4.2. --- charts/web3signer/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/web3signer/values.yaml b/charts/web3signer/values.yaml index b8d66e1..baf023c 100644 --- a/charts/web3signer/values.yaml +++ b/charts/web3signer/values.yaml @@ -27,7 +27,7 @@ dbKeystoreTableName: "" ## NOTE: the fetch-keys init container below still defaults to sync-keys 0.4.1, which does not ## understand the options here. A release that sets any of them must also pin -## initContainers[fetch-keys].image.tag to 0.5.0 or newer. The default is deliberately left +## initContainers[fetch-keys].image.tag to 0.4.2 or newer. The default is deliberately left ## alone: every release pins a chart version, so changing it would not affect anyone ## immediately, but it would silently bundle a sync-keys upgrade into whichever release next ## adopts this chart version. Image bumps should be their own decision. From 8416a4fb05c2fd7ef04483057614f8cd37412154 Mon Sep 17 00:00:00 2001 From: kenrick-g Date: Mon, 7 Sep 2026 22:04:45 +0800 Subject: [PATCH 08/12] fix: drop dbKeystoreAllowNoKeys, and require sync-keys 0.4.3 sync-keys removed --allow-no-keys, so rendering it would pass an option the CLI rejects. The refusal it overrode is unreachable for a correctly configured signer under one KOS database per namespace, and an empty table is tolerated without it -- which is the case that actually occurs. Also pins the required version at 0.4.3 and records that 0.4.2 must be skipped: it refuses a zero-row read unconditionally, which would stop QA's euw1/hoodi-2 from starting. Verified by render: a legacy release is byte-identical to origin/main apart from the chart version, cluster ids still render once per entry on both arg paths, and setting dbKeystoreAllowNoKeys now emits nothing. --- charts/web3signer/templates/_helpers.tpl | 3 --- charts/web3signer/values.yaml | 13 ++++--------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/charts/web3signer/templates/_helpers.tpl b/charts/web3signer/templates/_helpers.tpl index 4b0d70d..d42bc69 100644 --- a/charts/web3signer/templates/_helpers.tpl +++ b/charts/web3signer/templates/_helpers.tpl @@ -190,9 +190,6 @@ Create the name of the service account to use {{- if .root.Values.dbKeystoreAllClusters -}} {{- $fetchKeysFlags = concat $fetchKeysFlags (list "--all-clusters") -}} {{- end -}} - {{- if .root.Values.dbKeystoreAllowNoKeys -}} - {{- $fetchKeysFlags = concat $fetchKeysFlags (list "--allow-no-keys") -}} - {{- end -}} {{- end -}} - name: {{ .container.name }} diff --git a/charts/web3signer/values.yaml b/charts/web3signer/values.yaml index baf023c..b128d07 100644 --- a/charts/web3signer/values.yaml +++ b/charts/web3signer/values.yaml @@ -27,14 +27,14 @@ dbKeystoreTableName: "" ## NOTE: the fetch-keys init container below still defaults to sync-keys 0.4.1, which does not ## understand the options here. A release that sets any of them must also pin -## initContainers[fetch-keys].image.tag to 0.4.2 or newer. The default is deliberately left +## initContainers[fetch-keys].image.tag to 0.4.3 or newer. The default is deliberately left ## alone: every release pins a chart version, so changing it would not affect anyone ## immediately, but it would silently bundle a sync-keys upgrade into whichever release next ## adopts this chart version. Image bumps should be their own decision. ## -## When that default does move, a namespace whose keystore query returns no rows needs -## dbKeystoreAllowNoKeys in the same change, or its init container will fail. QA's -## euw1/hoodi-2 is one such namespace. +## 0.4.2 must be skipped: it refuses a zero-row read unconditionally, which stops a signer +## that legitimately serves no validators from starting. 0.4.3 tolerates an empty table and +## refuses only when the table holds keys that do not match the cluster asked for. ## ## Restrict fetched keys to these clusters. Required when the keystore table carries a ## client_cluster_id column, because that table holds more than one cluster's keys and @@ -49,11 +49,6 @@ dbKeystoreClientClusterIds: [] ## signer really does serve every cluster in that table, including ones added later. Prefer ## naming the clusters. Mutually exclusive with dbKeystoreClientClusterIds. dbKeystoreAllClusters: false -## Start with an empty keystore when the keystore table returns no rows, instead of failing -## the init container. Set this only for a signer that is deployed but serves no validators -## yet; otherwise a wrong cluster id or an unpopulated table should stop the pod rather than -## bring up a signer that cannot sign. -dbKeystoreAllowNoKeys: false ## The key for decrypting private keys is generated with stakewise-cli sync-db command ## Use this for plain text decryption key (legacy mode) From e876c0d270036eff515d84b4ebac12259f717226 Mon Sep 17 00:00:00 2001 From: kenrick-g Date: Tue, 8 Sep 2026 01:23:35 +0800 Subject: [PATCH 09/12] chore: release as 6.4.5 rather than a 6.5.0 minor The new values are inert unless set and the only behaviour change is the AWS KMS base64 fix, so a patch release is the honest signal. 6.4.3 is already published and main is at 6.4.4, so 6.4.5 is the next available patch. --- charts/web3signer/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/web3signer/Chart.yaml b/charts/web3signer/Chart.yaml index 412bd4a..86ff28d 100644 --- a/charts/web3signer/Chart.yaml +++ b/charts/web3signer/Chart.yaml @@ -15,7 +15,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 6.5.0 +version: 6.4.5 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to From 49039c8b2277909e58c79d5273d7ea9e4f5c625c Mon Sep 17 00:00:00 2001 From: kenrick-g Date: Tue, 8 Sep 2026 18:57:57 +0800 Subject: [PATCH 10/12] fix: fail the render when the decrypted-secret wrapper has nothing to exec An init container with usesDecryptedSecret and neither command nor args could not work: the wrapper replaces the image entrypoint with /bin/sh -c, so it cannot fall back to ENTRYPOINT the way the plain command/args path does, and it cannot exec the sync-keys flags on their own because the first word would be taken as the program name. That case emitted a container that echoed an error and exited 1, so a misconfiguration surfaced during a rollout instead of at template time. Fail the render instead. Both shipped init containers set command and args, so nothing that currently renders is affected. Also correct two comments that described behaviour inaccurately: - The KMS decrypt comment said a NUL byte truncated the key. It does not truncate: command substitution drops each NUL and strips trailing newlines, so a 32-byte key comes back one byte short per NUL. Verified directly -- 32 bytes in, 31 out for one NUL and 30 for two. The one-in-eight figure is right (1 - (255/256)^32 = 11.8%). - The values.yaml note said an older sync-keys "does not understand" the new options, which left open whether it ignores them. It does not: click exits 2 with "No such option", so the init container crashloops. Worth stating, because it means the flag cannot be silently dropped and leave a signer serving more keys than intended. --- charts/web3signer/templates/_helpers.tpl | 21 ++++++++++++++------- charts/web3signer/values.yaml | 12 ++++++++---- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/charts/web3signer/templates/_helpers.tpl b/charts/web3signer/templates/_helpers.tpl index d42bc69..c261b72 100644 --- a/charts/web3signer/templates/_helpers.tpl +++ b/charts/web3signer/templates/_helpers.tpl @@ -88,11 +88,13 @@ Create the name of the service account to use export AWS_SESSION_TOKEN=$(echo $CREDS | sed -n 's/.*"SessionToken": "\([^"]*\)".*/\1/p') {{- end }} echo "$ENCRYPTED_DECRYPTION_KEY" | base64 -d > /tmp/ciphertext.bin - # Leave the value base64-encoded. sync-keys base64-decodes DECRYPTION_KEY itself, and - # the Azure branch below likewise writes base64. Decoding here also lost bytes: raw key - # material does not survive a shell command substitution, so a key containing a null - # byte was silently truncated and sync-keys then failed with - # "Incorrect AES key length". That affects roughly one random 32-byte key in eight. + # Leave the value base64-encoded. sync-keys base64-decodes DECRYPTION_KEY itself in + # utils.str_to_bytes, and the Azure branch below likewise writes base64. Decoding here + # corrupted the key rather than truncating it: command substitution drops every NUL byte + # and strips trailing newlines, so a 32-byte key came back one byte short per NUL and + # sync-keys failed with "Incorrect AES key length". Measured, not inferred -- 32 bytes in, + # 31 out for one NUL, 30 for two. About one random 32-byte key in eight holds at least + # one NUL (1 - (255/256)^32 = 11.8%), which is why this looked intermittent. DECRYPTED=$(aws kms decrypt \ --region "$AWS_REGION" \ --ciphertext-blob fileb:///tmp/ciphertext.bin \ @@ -236,8 +238,13 @@ Create the name of the service account to use {{- else if .container.args }} exec {{ range $renderedArgs }}{{ . | quote }} {{ end }} {{- else }} - echo "Error: No command or args specified for container with usesDecryptedSecret" - exit 1 + {{- /* + Nothing to exec. This wrapper replaces the image entrypoint with /bin/sh -c, so unlike + the plain command/args path below it cannot fall back to the image ENTRYPOINT -- and it + cannot exec the flags alone, because the first word would be taken as the program name. + Fail the render instead of emitting a container that exits 1 during a rollout. + */ -}} + {{- fail (printf "web3signer: init container %q sets usesDecryptedSecret but declares neither command nor args. The decrypted-secret wrapper replaces the image entrypoint, so one of them must be set explicitly." .container.name) }} {{- end }} {{- else }} {{- if .container.command }} diff --git a/charts/web3signer/values.yaml b/charts/web3signer/values.yaml index b128d07..e8b75e5 100644 --- a/charts/web3signer/values.yaml +++ b/charts/web3signer/values.yaml @@ -27,10 +27,14 @@ dbKeystoreTableName: "" ## NOTE: the fetch-keys init container below still defaults to sync-keys 0.4.1, which does not ## understand the options here. A release that sets any of them must also pin -## initContainers[fetch-keys].image.tag to 0.4.3 or newer. The default is deliberately left -## alone: every release pins a chart version, so changing it would not affect anyone -## immediately, but it would silently bundle a sync-keys upgrade into whichever release next -## adopts this chart version. Image bumps should be their own decision. +## initContainers[fetch-keys].image.tag to 0.4.3 or newer. An older image fails loudly rather +## than quietly ignoring the flag -- click exits 2 with "No such option: --client-cluster-id", +## so the init container crashloops and the signer never starts. That is the safe direction: +## the flag cannot be silently dropped and leave a signer serving more keys than intended. +## +## The default is deliberately left alone: every release pins a chart version, so changing it +## would not affect anyone immediately, but it would silently bundle a sync-keys upgrade into +## whichever release next adopts this chart version. Image bumps should be their own decision. ## ## 0.4.2 must be skipped: it refuses a zero-row read unconditionally, which stops a signer ## that legitimately serves no validators from starting. 0.4.3 tolerates an empty table and From f3e2625d46a89ad4e6ccdf7303a8220a8d543134 Mon Sep 17 00:00:00 2001 From: kenrick-g Date: Tue, 8 Sep 2026 20:08:44 +0800 Subject: [PATCH 11/12] fix: assert the DECRYPTION_KEY encoding where the provider branches converge The AWS branch wrote raw key bytes while the Azure branch wrote base64, and nothing recorded which one the consumer needs. sync-keys base64-decodes what it reads, so base64 is the answer and the AWS branch was wrong. Separating the two branches further would not have caught that; the encoding is a property of the file they share, so the check belongs at the one point where they meet. Reject a value that does not decode to a 16/24/32-byte key before writing it. That covers both ways this has gone wrong -- raw bytes instead of base64 (decodes to 0) and a key shortened by the shell eating a NUL (decodes to 31) -- and it fails at the cause instead of as an opaque error inside sync-keys. Verified against the live Azure value, which is 44 characters of standard base64 decoding to 32 bytes, so the assertion passes for the deployment that already works. Also revert two changes from the previous commit: - The render-time fail for an init container with neither command nor args is reverted. Both shipped init containers set both, so it was unreachable, and the existing runtime message was already clear. It only moved when the operator finds out, at the cost of a behaviour change in a shared chart. - The decrypt comment claimed the symptom was "Incorrect AES key length" on roughly one key in eight. That was wrong. Writing raw bytes where base64 was expected failed for every key, and earlier than that message. The NUL story is a genuine second hazard but a hypothetical one, so it no longer reads as the observed failure. --- charts/web3signer/templates/_helpers.tpl | 42 ++++++++++++++++-------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/charts/web3signer/templates/_helpers.tpl b/charts/web3signer/templates/_helpers.tpl index c261b72..298aa63 100644 --- a/charts/web3signer/templates/_helpers.tpl +++ b/charts/web3signer/templates/_helpers.tpl @@ -88,13 +88,16 @@ Create the name of the service account to use export AWS_SESSION_TOKEN=$(echo $CREDS | sed -n 's/.*"SessionToken": "\([^"]*\)".*/\1/p') {{- end }} echo "$ENCRYPTED_DECRYPTION_KEY" | base64 -d > /tmp/ciphertext.bin - # Leave the value base64-encoded. sync-keys base64-decodes DECRYPTION_KEY itself in - # utils.str_to_bytes, and the Azure branch below likewise writes base64. Decoding here - # corrupted the key rather than truncating it: command substitution drops every NUL byte - # and strips trailing newlines, so a 32-byte key came back one byte short per NUL and - # sync-keys failed with "Incorrect AES key length". Measured, not inferred -- 32 bytes in, - # 31 out for one NUL, 30 for two. About one random 32-byte key in eight holds at least - # one NUL (1 - (255/256)^32 = 11.8%), which is why this looked intermittent. + # Leave the value base64-encoded: the contract asserted below is that + # /decrypted-secrets/DECRYPTION_KEY holds base64, never raw key bytes. Decoding here made + # this branch write raw bytes, which sync-keys then base64-decoded again + # (utils.str_to_bytes), so every key was rejected -- not an occasional one. The Azure + # branch never had the bug because it writes the base64 its decrypt call returns. + # + # Raw bytes could not have worked anyway: the value passes through two command + # substitutions before sync-keys sees it, and the shell drops every NUL byte and strips + # trailing newlines, so a 32-byte key arrives one byte short per NUL. Base64 is ASCII and + # survives both. DECRYPTED=$(aws kms decrypt \ --region "$AWS_REGION" \ --ciphertext-blob fileb:///tmp/ciphertext.bin \ @@ -142,6 +145,22 @@ Create the name of the service account to use {{- end }} --query result --output tsv) {{- end }} + # Single point where every provider branch converges, so this is where the encoding + # contract belongs. Both decrypt calls already return standard base64 and sync-keys + # base64-decodes what it reads, so a value that does not decode to a valid AES key length + # means a branch above got it wrong. Checking it here fails at the cause with a clear + # message rather than as an opaque error inside sync-keys, and it covers both ways this + # has gone wrong: raw bytes instead of base64 (decodes to 0) and a key shortened by the + # shell eating a NUL (decodes to 31). + DECODED_LEN=$(printf '%s' "$DECRYPTED" | base64 -d 2>/dev/null | wc -c | tr -d ' ') + case "$DECODED_LEN" in + 16|24|32) ;; + *) + echo "ERROR: decrypted DECRYPTION_KEY is not base64 of a 16/24/32-byte AES key" >&2 + echo "ERROR: decoded to ${DECODED_LEN} bytes; expected base64 text, not raw key bytes" >&2 + exit 1 + ;; + esac echo -n "$DECRYPTED" > /decrypted-secrets/DECRYPTION_KEY {{- if and (eq $provider "azure") .Values.encryptedDecryptionKey.useKeyVaultSecrets }} echo -n "$DB_PASSWORD" > /decrypted-secrets/DB_PASSWORD @@ -238,13 +257,8 @@ Create the name of the service account to use {{- else if .container.args }} exec {{ range $renderedArgs }}{{ . | quote }} {{ end }} {{- else }} - {{- /* - Nothing to exec. This wrapper replaces the image entrypoint with /bin/sh -c, so unlike - the plain command/args path below it cannot fall back to the image ENTRYPOINT -- and it - cannot exec the flags alone, because the first word would be taken as the program name. - Fail the render instead of emitting a container that exits 1 during a rollout. - */ -}} - {{- fail (printf "web3signer: init container %q sets usesDecryptedSecret but declares neither command nor args. The decrypted-secret wrapper replaces the image entrypoint, so one of them must be set explicitly." .container.name) }} + echo "Error: No command or args specified for container with usesDecryptedSecret" + exit 1 {{- end }} {{- else }} {{- if .container.command }} From 34a2531f708e3e519543b9dce9ee5d5162f88e78 Mon Sep 17 00:00:00 2001 From: kenrick-g Date: Wed, 9 Sep 2026 13:45:01 +0800 Subject: [PATCH 12/12] fix: drop the cluster-selection values, keep the encoding work These values were added for a migration that turns out not to need them, and the guard behind them checks the wrong property. sync-keys refuses only a zero-row read, so naming a subset of a signer's clusters returns rows and the check stays quiet while the signer comes up Ready with a partial keystore -- the omitted cluster's validators stop signing with nothing to indicate it. The two settings do not order: naming clusters guards against reading the wrong database but allows an incomplete set, and the all-clusters override is the reverse. A control that makes the operator choose which failure they would rather have is not a control. Where one database holds one signer's keys, an unfiltered read is already the correct read, so nothing is lost by removing them. The property actually worth checking -- that the keystore holds the keys this signer should serve -- needs an expected count, which belongs in a post-deploy check rather than in values that can be typed wrong. values.yaml is byte-identical to main again. Kept from the previous commits: - the AWS KMS base64 fix, which is a real defect on that branch: it wrote raw key bytes where sync-keys base64-decodes, so it failed for every key. Now verified end to end against real KMS on both an ordinary key and one containing a NUL byte, round-tripping to the original 32 bytes. - the encoding assertion where the provider branches converge, which rejects raw bytes, a NUL-shortened key and an empty decrypt. - building the fetch-keys flags in one place, so a flag added later cannot reach only one of the two arg-rendering paths. The chart version bump stays: the base64 fix is a behaviour change and consumers pin chart versions. --- charts/web3signer/templates/_helpers.tpl | 16 ++----------- charts/web3signer/values.yaml | 29 ------------------------ 2 files changed, 2 insertions(+), 43 deletions(-) diff --git a/charts/web3signer/templates/_helpers.tpl b/charts/web3signer/templates/_helpers.tpl index 298aa63..ef49cc3 100644 --- a/charts/web3signer/templates/_helpers.tpl +++ b/charts/web3signer/templates/_helpers.tpl @@ -191,26 +191,14 @@ Create the name of the service account to use {{- /* Extra sync-keys flags for the fetch-keys container. Built once here because there are two arg-rendering paths below -- the encryptedDecryptionKey one that execs through a shell, and - the plain command/args one -- and a flag added to only one of them is silently dropped on - every release using the other. + the plain command/args one -- and a flag appended to only one of them is silently dropped on + every release that happens to use the other. */ -}} {{- $fetchKeysFlags := list -}} {{- if $isFetchKeys -}} - {{- if and .root.Values.dbKeystoreClientClusterIds .root.Values.dbKeystoreAllClusters -}} - {{- fail "web3signer: set dbKeystoreClientClusterIds or dbKeystoreAllClusters, not both" -}} - {{- end -}} {{- if .root.Values.dbKeystoreTableName -}} {{- $fetchKeysFlags = concat $fetchKeysFlags (list "--table-name" .root.Values.dbKeystoreTableName) -}} {{- end -}} - {{- range .root.Values.dbKeystoreClientClusterIds -}} - {{- if not . -}} - {{- fail "web3signer: dbKeystoreClientClusterIds must not contain an empty value" -}} - {{- end -}} - {{- $fetchKeysFlags = concat $fetchKeysFlags (list "--client-cluster-id" .) -}} - {{- end -}} - {{- if .root.Values.dbKeystoreAllClusters -}} - {{- $fetchKeysFlags = concat $fetchKeysFlags (list "--all-clusters") -}} - {{- end -}} {{- end -}} - name: {{ .container.name }} diff --git a/charts/web3signer/values.yaml b/charts/web3signer/values.yaml index e8b75e5..25c226f 100644 --- a/charts/web3signer/values.yaml +++ b/charts/web3signer/values.yaml @@ -25,35 +25,6 @@ dbKeystoreUrl: "" ## dbKeystoreTableName: "" -## NOTE: the fetch-keys init container below still defaults to sync-keys 0.4.1, which does not -## understand the options here. A release that sets any of them must also pin -## initContainers[fetch-keys].image.tag to 0.4.3 or newer. An older image fails loudly rather -## than quietly ignoring the flag -- click exits 2 with "No such option: --client-cluster-id", -## so the init container crashloops and the signer never starts. That is the safe direction: -## the flag cannot be silently dropped and leave a signer serving more keys than intended. -## -## The default is deliberately left alone: every release pins a chart version, so changing it -## would not affect anyone immediately, but it would silently bundle a sync-keys upgrade into -## whichever release next adopts this chart version. Image bumps should be their own decision. -## -## 0.4.2 must be skipped: it refuses a zero-row read unconditionally, which stops a signer -## that legitimately serves no validators from starting. 0.4.3 tolerates an empty table and -## refuses only when the table holds keys that do not match the cluster asked for. -## -## Restrict fetched keys to these clusters. Required when the keystore table carries a -## client_cluster_id column, because that table holds more than one cluster's keys and -## sync-keys otherwise refuses to run. Leave empty for the agent-managed table, which has no -## such column. -## -## A list, because one signer can legitimately serve several clusters. Naming them is -## preferable to dbKeystoreAllClusters: the set is closed, so a cluster added to the same -## database later is not served unless it is added here. -dbKeystoreClientClusterIds: [] -## Deliberately load every cluster's keys from a cluster-scoped table. Only correct when this -## signer really does serve every cluster in that table, including ones added later. Prefer -## naming the clusters. Mutually exclusive with dbKeystoreClientClusterIds. -dbKeystoreAllClusters: false - ## The key for decrypting private keys is generated with stakewise-cli sync-db command ## Use this for plain text decryption key (legacy mode) ##