feat: Add signer abstraction via registry interface#7938
Open
nambrot-agent wants to merge 1 commit intomainfrom
Open
feat: Add signer abstraction via registry interface#7938nambrot-agent wants to merge 1 commit intomainfrom
nambrot-agent wants to merge 1 commit intomainfrom
Conversation
Implement signer abstraction that allows CLI users to configure signers through registry files or URIs instead of passing private keys via --key. SDK Changes (typescript/sdk/src/signers/): - config.ts: Define signer types (rawKey, turnkey, gcpSecret, foundryKeystore) with Zod schemas and hierarchical defaults (chain > protocol > default) - SignerFactory.ts: Create ethers Signers from config objects - Support for raw keys (direct or env var) - GCP Secret Manager (via gcloud CLI, no SDK dependency) - Foundry keystores (with ETH_PASSWORD support per Foundry standard) - Turnkey secure enclaves - extractPrivateKey() for exporting keys to external tools - index.ts: Export all signer types and factory CLI Changes: - context.ts: Load signer config from registry via getSignerConfiguration() - MultiProtocolSignerManager.ts: Resolve signers from registry with fallback Resolution order: registry (chain > protocol > default) -> strategy -> --key - registry.ts: Add 'hyperlane registry signer-key' command for extracting private keys for use with external tools (Foundry, etc.) New Commands: - hyperlane registry signer-key: Extract private key from registry signer --name: Get specific named signer --chain: Get signer for specific chain --address-only: Output only the address Documentation: - SIGNER_REGISTRY.md: Comprehensive guide for signer configuration This feature works with hyperlane-xyz/hyperlane-registry PR for signer registry URIs (gcp://, foundry://).
nambrot-agent
requested review from
Xaroz,
antigremlin,
ltyu,
paulbalaji,
xeno097 and
yorhodes
as code owners
Open
4 tasks
Contributor
📝 WalkthroughWalkthroughThis PR introduces a comprehensive signer registry system enabling CLI signers to be configured via registry files and URIs instead of direct private keys. It adds documentation, a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Command
participant Registry as Signer Registry
participant Factory as SignerFactory
participant Provider as Ethers Provider
participant Signer as Ethers Signer
CLI->>Registry: resolveSignerConfig(name, chain)
Note over Registry: Check chain-specific > protocol-specific > default
alt Registry has config
Registry-->>CLI: SignerConfig
CLI->>Factory: createSigner(config, provider)
alt Config type is RAW_KEY
Factory->>Factory: resolvePrivateKey (direct or env var)
Factory->>Signer: new Wallet(privateKey)
Signer-->>Factory: Wallet instance
else Config type is GCP_SECRET
Factory->>Registry: fetchGCPSecret(project, name)
Registry-->>Factory: secret string
Factory->>Signer: new Wallet(secret)
Signer-->>Factory: Wallet instance
else Config type is TURNKEY/FOUNDRY
Factory->>Provider: createSigner (external creation)
Provider-->>Factory: Signer instance
end
Factory-->>CLI: Signer
else Registry lacks config
Registry-->>CLI: null
CLI->>CLI: Fall back to --key or default strategy
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@typescript/cli/SIGNER_REGISTRY.md`:
- Around line 84-86: The README has a bare URL in the Requirements section;
update SIGNER_REGISTRY.md to wrap the URL
"https://cloud.google.com/sdk/docs/install" in proper Markdown (either angle
brackets like <https://cloud.google.com/sdk/docs/install> or use link text like
[gcloud CLI installation](https://cloud.google.com/sdk/docs/install)) so linters
won't flag the bare URL and rendering stays consistent.
In `@typescript/cli/src/commands/registry.ts`:
- Around line 295-321: The resolution currently checks defaults.chains and then
defaults.default but skips protocol-level defaults; update the resolution in
registry.ts (around signerConfig.defaults, signerOrRef and signerSource) to
derive the protocol for the provided chain (via your chain metadata lookup or
existing chain->protocol mapping) and then check defaults.protocols[protocol]
before falling back to defaults.default; set signerOrRef and signerSource
accordingly (e.g., "protocol '<protocol>' default") when a protocol-level signer
is chosen.
In `@typescript/cli/src/tests/ethereum/signer-registry.e2e-test.ts`:
- Around line 245-283: The test "should prefer registry signer over --key when
both are provided" currently never passes a --key flag so it doesn't verify
precedence; update the command invocation inside that it(...) block to add a
deliberately invalid --key value (e.g., a bad hex private key string) to the
shell command constructed by localTestRunCmdPrefix() so the command would fail
if the CLI used the flag; keep the same assertions (expect exitCode === 0 and
output includes 'Message ID:', 'Created signer for chain', and 'from registry
configuration') and leave cleanup of `${SIGNERS_DIR}/preferred.yaml` as-is.
- Around line 49-83: The test leaves a persistent signer config file
(writeYamlOrJson(`${SIGNERS_DIR}/default.yaml`, ...)) which can leak into other
tests; update the test to isolate signer configs by writing the signer file into
a per-test temporary directory (or remove the file in an afterEach/try/finally
cleanup) and point the command to that temp signer registry path (replace uses
of SIGNERS_DIR/SIGNER_REGISTRY_PATH accordingly), ensuring writeYamlOrJson's
output is cleaned up so default.yaml cannot affect subsequent tests.
In `@typescript/sdk/src/signers/config.ts`:
- Around line 25-31: RawKeySignerConfigSchema currently allows an empty config
because both privateKey and privateKeyEnvVar are optional; update
RawKeySignerConfigSchema (the z.object with type: z.literal(SignerType.RAW_KEY),
privateKey, privateKeyEnvVar) to add a zod refinement (use superRefine) that
checks at least one of privateKey or privateKeyEnvVar is present and emits a
clear error path (e.g., add errors for privateKeyEnvVar or privateKey) when
neither is provided so validation fails early instead of at runtime.
In `@typescript/sdk/src/signers/SignerFactory.ts`:
- Around line 319-376: The fetchGCPSecret function currently builds a shell
command and uses exec, which permits shell injection; replace the dynamic
command string and promisified exec with promisified execFile (imported from
'child_process') and pass gcloud as the executable with an args array like
['secrets', 'versions', 'access', 'latest', '--secret', secretName, '--project',
project] (preserve the maxBuffer option), then consume stdout/stderr the same
way and keep the existing error parsing/rewriting logic so all the current
permission/auth/not-found checks remain intact; ensure you update the variable
names (e.g., execAsync -> execFileAsync) and maintain logger.debug showing
project/secretName without executing them in a shell.
🧹 Nitpick comments (4)
typescript/sdk/src/signers/SignerFactory.test.ts (1)
92-108: TURNKEY test creates signer but doesn't validate it works.The test creates a signer with dummy credentials and only checks it exists. That's fine for unit testing the factory's ability to instantiate, but maybe worth a wee comment explaining the limitation.
The dummy public key
'0x' + '00'.repeat(33)works for the test, but it's not a valid compressed public key format (should be 33 bytes starting with 0x02 or 0x03 for compressed keys).💡 Consider using a more realistic dummy key format
- publicKey: '0x' + '00'.repeat(33), // Dummy compressed public key + publicKey: '0x02' + '00'.repeat(32), // Dummy compressed public key (33 bytes with valid prefix)typescript/cli/src/context/strategies/signer/MultiProtocolSignerManager.ts (1)
247-258: Consider logging at debug level instead of info for successful creation.The info-level log on line 251 will be emitted for every chain where a registry signer is used. If someone has many chains configured, this could be noisy. Debug level might be more appropriate here.
The warning on failure (line 254) is appropriate since it indicates something the user should investigate.
💬 Consider using debug level for success message
const signer = await SignerFactory.createSigner(resolvedConfig, provider); - this.logger.info(`Created signer for chain ${chain} from registry configuration`); + this.logger.debug(`Created signer for chain ${chain} from registry configuration`); return signer as TypedSigner;typescript/sdk/src/signers/config.ts (1)
136-147: UseChainMapfor per-chain defaults.
Thechainsfield represents a per-chain mapping. Type it asChainMap<SignerOrRef>in the inferredSignerDefaultstype to align with SDK conventions and preserve chain-name guarantees across the surface. While the Zod schema validates correctly as-is, manually typing the result to useChainMapimproves consistency with other per-chain configurations in the SDK.typescript/cli/src/commands/registry.ts (1)
275-348: TyperesolvedConfigandsignerOrRefto eliminate theanycast.These variables flow through discriminated union logic that TypeScript can properly narrow with
isSignerRef, but the implicit typing forces the(resolvedConfig as any).accountNamecast. DeclareresolvedConfig: SignerConfigandsignerOrRef: SignerOrReffrom@hyperlane-xyz/sdk—the union will narrow cleanly based on thetypefield, and the cast becomes unnecessary.♻️ Type-safe adjustment
import { EXTRACTABLE_SIGNER_TYPES, SignerFactory, SignerType, isSignerRef, + type SignerConfig, + type SignerOrRef, } from '@hyperlane-xyz/sdk'; @@ - let resolvedConfig; + let resolvedConfig: SignerConfig; let signerSource: string; @@ - let signerOrRef; + let signerOrRef: SignerOrRef; @@ - ` cast wallet decrypt-keystore ${(resolvedConfig as any).accountName}\n`, + ` cast wallet decrypt-keystore ${resolvedConfig.accountName}\n`,
Comment on lines
+84
to
+86
| Requirements: | ||
| - Install the `gcloud` CLI: https://cloud.google.com/sdk/docs/install | ||
| - Authenticate: `gcloud auth application-default login` |
Contributor
There was a problem hiding this comment.
Bare URL should be wrapped in proper markdown syntax.
The linter's got a point here - bare URLs can cause rendering issues in some markdown parsers. Nothing major, just a wee formatting thing.
📝 Wrap URL in angle brackets or use link syntax
Requirements:
-- Install the `gcloud` CLI: https://cloud.google.com/sdk/docs/install
+- Install the `gcloud` CLI: <https://cloud.google.com/sdk/docs/install>
- Authenticate: `gcloud auth application-default login`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Requirements: | |
| - Install the `gcloud` CLI: https://cloud.google.com/sdk/docs/install | |
| - Authenticate: `gcloud auth application-default login` | |
| Requirements: | |
| - Install the `gcloud` CLI: <https://cloud.google.com/sdk/docs/install> | |
| - Authenticate: `gcloud auth application-default login` |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
85-85: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In `@typescript/cli/SIGNER_REGISTRY.md` around lines 84 - 86, The README has a
bare URL in the Requirements section; update SIGNER_REGISTRY.md to wrap the URL
"https://cloud.google.com/sdk/docs/install" in proper Markdown (either angle
brackets like <https://cloud.google.com/sdk/docs/install> or use link text like
[gcloud CLI installation](https://cloud.google.com/sdk/docs/install)) so linters
won't flag the bare URL and rendering stays consistent.
Comment on lines
+295
to
+321
| // Get the default signer (optionally for a specific chain) | ||
| const defaults = signerConfig.defaults; | ||
| if (!defaults) { | ||
| errorRed( | ||
| '❌ No default signer configured in registry.\n' + | ||
| 'Specify a signer name with --name or configure defaults in the registry.', | ||
| ); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| // Resolution order: chain > protocol > default | ||
| let signerOrRef; | ||
| if (chain && defaults.chains?.[chain]) { | ||
| signerOrRef = defaults.chains[chain]; | ||
| signerSource = `chain '${chain}' default`; | ||
| } else if (defaults.default) { | ||
| signerOrRef = defaults.default; | ||
| signerSource = 'default signer'; | ||
| } else { | ||
| errorRed( | ||
| '❌ No default signer configured.\n' + | ||
| (chain | ||
| ? `No signer configured for chain '${chain}' or as default.` | ||
| : 'Specify --chain or --name to select a signer.'), | ||
| ); | ||
| process.exit(1); | ||
| } |
Contributor
There was a problem hiding this comment.
Protocol-level defaults aren’t resolved.
The comment says chain → protocol → default, but defaults.protocols is skipped. That means protocol-only configs fail even when --chain is provided. Consider resolving protocol from chain metadata before falling back to default.
🔧 Suggested resolution order
- // Resolution order: chain > protocol > default
- let signerOrRef;
- if (chain && defaults.chains?.[chain]) {
- signerOrRef = defaults.chains[chain];
- signerSource = `chain '${chain}' default`;
- } else if (defaults.default) {
- signerOrRef = defaults.default;
- signerSource = 'default signer';
- } else {
- errorRed(
- '❌ No default signer configured.\n' +
- (chain
- ? `No signer configured for chain '${chain}' or as default.`
- : 'Specify --chain or --name to select a signer.'),
- );
- process.exit(1);
- }
+ // Resolution order: chain > protocol > default
+ let signerOrRef;
+ if (chain) {
+ const chainMeta = context.chainMetadata[chain];
+ if (!chainMeta) {
+ errorRed(`❌ Unknown chain '${chain}'.`);
+ process.exit(1);
+ }
+ if (defaults.chains?.[chain]) {
+ signerOrRef = defaults.chains[chain];
+ signerSource = `chain '${chain}' default`;
+ } else if (defaults.protocols?.[chainMeta.protocol]) {
+ signerOrRef = defaults.protocols[chainMeta.protocol];
+ signerSource = `protocol '${chainMeta.protocol}' default`;
+ } else if (defaults.default) {
+ signerOrRef = defaults.default;
+ signerSource = 'default signer';
+ }
+ } else if (defaults.default) {
+ signerOrRef = defaults.default;
+ signerSource = 'default signer';
+ }
+
+ if (!signerOrRef) {
+ errorRed(
+ '❌ No default signer configured.\n' +
+ (chain
+ ? `No signer configured for chain '${chain}' or as default.`
+ : 'Specify --chain or --name to select a signer.'),
+ );
+ process.exit(1);
+ }🤖 Prompt for AI Agents
In `@typescript/cli/src/commands/registry.ts` around lines 295 - 321, The
resolution currently checks defaults.chains and then defaults.default but skips
protocol-level defaults; update the resolution in registry.ts (around
signerConfig.defaults, signerOrRef and signerSource) to derive the protocol for
the provided chain (via your chain metadata lookup or existing chain->protocol
mapping) and then check defaults.protocols[protocol] before falling back to
defaults.default; set signerOrRef and signerSource accordingly (e.g., "protocol
'<protocol>' default") when a protocol-level signer is chosen.
Comment on lines
+49
to
+83
| describe('rawKey signer from registry', () => { | ||
| it('should send message using signer from registry config file', async () => { | ||
| // Create signer configuration with rawKey type | ||
| const signerConfig = { | ||
| signers: { | ||
| deployer: { | ||
| type: 'rawKey', | ||
| privateKey: ANVIL_KEY, | ||
| }, | ||
| }, | ||
| defaults: { | ||
| default: { ref: 'deployer' }, | ||
| }, | ||
| }; | ||
|
|
||
| writeYamlOrJson(`${SIGNERS_DIR}/default.yaml`, signerConfig); | ||
|
|
||
| // Run send message command with registry that includes signer config | ||
| // Note: We provide both the anvil registry (for chain metadata) and | ||
| // the signer registry (for signer config) | ||
| const { exitCode, stdout, stderr } = | ||
| await $`${localTestRunCmdPrefix()} hyperlane send message \ | ||
| --registry ${REGISTRY_PATH} \ | ||
| --registry ${SIGNER_REGISTRY_PATH} \ | ||
| --origin ${CHAIN_NAME_2} \ | ||
| --destination ${CHAIN_NAME_3} \ | ||
| --verbosity debug \ | ||
| --quick \ | ||
| --yes`.nothrow(); | ||
|
|
||
| const output = stdout + stderr; | ||
| expect(exitCode, `Command failed with output: ${output}`).to.equal(0); | ||
| expect(output).to.include('Message ID:'); | ||
| expect(output).to.include(`Sent message from ${CHAIN_NAME_2}`); | ||
| }); |
Contributor
There was a problem hiding this comment.
Isolate signer configs between tests.
default.yaml sticks around and can override later defaults depending on merge order, which can make later tests flaky. Clean it up per-test or use per-test temp dirs.
🔧 Suggested cleanup
writeYamlOrJson(`${SIGNERS_DIR}/default.yaml`, signerConfig);
- // Run send message command with registry that includes signer config
- // Note: We provide both the anvil registry (for chain metadata) and
- // the signer registry (for signer config)
- const { exitCode, stdout, stderr } =
- await $`${localTestRunCmdPrefix()} hyperlane send message \
- --registry ${REGISTRY_PATH} \
- --registry ${SIGNER_REGISTRY_PATH} \
- --origin ${CHAIN_NAME_2} \
- --destination ${CHAIN_NAME_3} \
- --verbosity debug \
- --quick \
- --yes`.nothrow();
-
- const output = stdout + stderr;
- expect(exitCode, `Command failed with output: ${output}`).to.equal(0);
- expect(output).to.include('Message ID:');
- expect(output).to.include(`Sent message from ${CHAIN_NAME_2}`);
+ try {
+ // Run send message command with registry that includes signer config
+ // Note: We provide both the anvil registry (for chain metadata) and
+ // the signer registry (for signer config)
+ const { exitCode, stdout, stderr } =
+ await $`${localTestRunCmdPrefix()} hyperlane send message \
+ --registry ${REGISTRY_PATH} \
+ --registry ${SIGNER_REGISTRY_PATH} \
+ --origin ${CHAIN_NAME_2} \
+ --destination ${CHAIN_NAME_3} \
+ --verbosity debug \
+ --quick \
+ --yes`.nothrow();
+
+ const output = stdout + stderr;
+ expect(exitCode, `Command failed with output: ${output}`).to.equal(0);
+ expect(output).to.include('Message ID:');
+ expect(output).to.include(`Sent message from ${CHAIN_NAME_2}`);
+ } finally {
+ fs.unlinkSync(`${SIGNERS_DIR}/default.yaml`);
+ }🤖 Prompt for AI Agents
In `@typescript/cli/src/tests/ethereum/signer-registry.e2e-test.ts` around lines
49 - 83, The test leaves a persistent signer config file
(writeYamlOrJson(`${SIGNERS_DIR}/default.yaml`, ...)) which can leak into other
tests; update the test to isolate signer configs by writing the signer file into
a per-test temporary directory (or remove the file in an afterEach/try/finally
cleanup) and point the command to that temp signer registry path (replace uses
of SIGNERS_DIR/SIGNER_REGISTRY_PATH accordingly), ensuring writeYamlOrJson's
output is cleaned up so default.yaml cannot affect subsequent tests.
Comment on lines
+245
to
+283
| it('should prefer registry signer over --key when both are provided', async () => { | ||
| // Create signer config | ||
| const signerConfig = { | ||
| signers: { | ||
| 'registry-signer': { | ||
| type: 'rawKey', | ||
| privateKey: ANVIL_KEY, | ||
| }, | ||
| }, | ||
| defaults: { | ||
| default: { ref: 'registry-signer' }, | ||
| }, | ||
| }; | ||
| writeYamlOrJson(`${SIGNERS_DIR}/preferred.yaml`, signerConfig); | ||
|
|
||
| try { | ||
| // Provide a different (invalid) key via --key | ||
| // If registry signer is preferred, command should succeed | ||
| // If --key is used, it would fail due to invalid key | ||
| const { exitCode, stdout, stderr } = | ||
| await $`${localTestRunCmdPrefix()} hyperlane send message \ | ||
| --registry ${REGISTRY_PATH} \ | ||
| --registry ${SIGNER_REGISTRY_PATH} \ | ||
| --origin ${CHAIN_NAME_2} \ | ||
| --destination ${CHAIN_NAME_3} \ | ||
| --verbosity debug \ | ||
| --quick \ | ||
| --yes`.nothrow(); | ||
|
|
||
| const output = stdout + stderr; | ||
| // The registry signer should be used, so the command should succeed | ||
| expect(exitCode, `Command failed with output: ${output}`).to.equal(0); | ||
| expect(output).to.include('Message ID:'); | ||
| expect(output).to.include('Created signer for chain'); | ||
| expect(output).to.include('from registry configuration'); | ||
| } finally { | ||
| fs.unlinkSync(`${SIGNERS_DIR}/preferred.yaml`); | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
Test doesn’t actually pass --key yet.
The command omits --key, so it never verifies precedence. Add an invalid key to ensure the registry signer is truly preferred.
🔧 Minimal fix
try {
// Provide a different (invalid) key via --key
// If registry signer is preferred, command should succeed
// If --key is used, it would fail due to invalid key
+ const INVALID_KEY = '0x123';
const { exitCode, stdout, stderr } =
await $`${localTestRunCmdPrefix()} hyperlane send message \
--registry ${REGISTRY_PATH} \
--registry ${SIGNER_REGISTRY_PATH} \
--origin ${CHAIN_NAME_2} \
--destination ${CHAIN_NAME_3} \
+ --key ${INVALID_KEY} \
--verbosity debug \
--quick \
--yes`.nothrow();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('should prefer registry signer over --key when both are provided', async () => { | |
| // Create signer config | |
| const signerConfig = { | |
| signers: { | |
| 'registry-signer': { | |
| type: 'rawKey', | |
| privateKey: ANVIL_KEY, | |
| }, | |
| }, | |
| defaults: { | |
| default: { ref: 'registry-signer' }, | |
| }, | |
| }; | |
| writeYamlOrJson(`${SIGNERS_DIR}/preferred.yaml`, signerConfig); | |
| try { | |
| // Provide a different (invalid) key via --key | |
| // If registry signer is preferred, command should succeed | |
| // If --key is used, it would fail due to invalid key | |
| const { exitCode, stdout, stderr } = | |
| await $`${localTestRunCmdPrefix()} hyperlane send message \ | |
| --registry ${REGISTRY_PATH} \ | |
| --registry ${SIGNER_REGISTRY_PATH} \ | |
| --origin ${CHAIN_NAME_2} \ | |
| --destination ${CHAIN_NAME_3} \ | |
| --verbosity debug \ | |
| --quick \ | |
| --yes`.nothrow(); | |
| const output = stdout + stderr; | |
| // The registry signer should be used, so the command should succeed | |
| expect(exitCode, `Command failed with output: ${output}`).to.equal(0); | |
| expect(output).to.include('Message ID:'); | |
| expect(output).to.include('Created signer for chain'); | |
| expect(output).to.include('from registry configuration'); | |
| } finally { | |
| fs.unlinkSync(`${SIGNERS_DIR}/preferred.yaml`); | |
| } | |
| }); | |
| it('should prefer registry signer over --key when both are provided', async () => { | |
| // Create signer config | |
| const signerConfig = { | |
| signers: { | |
| 'registry-signer': { | |
| type: 'rawKey', | |
| privateKey: ANVIL_KEY, | |
| }, | |
| }, | |
| defaults: { | |
| default: { ref: 'registry-signer' }, | |
| }, | |
| }; | |
| writeYamlOrJson(`${SIGNERS_DIR}/preferred.yaml`, signerConfig); | |
| try { | |
| // Provide a different (invalid) key via --key | |
| // If registry signer is preferred, command should succeed | |
| // If --key is used, it would fail due to invalid key | |
| const INVALID_KEY = '0x123'; | |
| const { exitCode, stdout, stderr } = | |
| await $`${localTestRunCmdPrefix()} hyperlane send message \ | |
| --registry ${REGISTRY_PATH} \ | |
| --registry ${SIGNER_REGISTRY_PATH} \ | |
| --origin ${CHAIN_NAME_2} \ | |
| --destination ${CHAIN_NAME_3} \ | |
| --key ${INVALID_KEY} \ | |
| --verbosity debug \ | |
| --quick \ | |
| --yes`.nothrow(); | |
| const output = stdout + stderr; | |
| // The registry signer should be used, so the command should succeed | |
| expect(exitCode, `Command failed with output: ${output}`).to.equal(0); | |
| expect(output).to.include('Message ID:'); | |
| expect(output).to.include('Created signer for chain'); | |
| expect(output).to.include('from registry configuration'); | |
| } finally { | |
| fs.unlinkSync(`${SIGNERS_DIR}/preferred.yaml`); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In `@typescript/cli/src/tests/ethereum/signer-registry.e2e-test.ts` around lines
245 - 283, The test "should prefer registry signer over --key when both are
provided" currently never passes a --key flag so it doesn't verify precedence;
update the command invocation inside that it(...) block to add a deliberately
invalid --key value (e.g., a bad hex private key string) to the shell command
constructed by localTestRunCmdPrefix() so the command would fail if the CLI used
the flag; keep the same assertions (expect exitCode === 0 and output includes
'Message ID:', 'Created signer for chain', and 'from registry configuration')
and leave cleanup of `${SIGNERS_DIR}/preferred.yaml` as-is.
Comment on lines
+25
to
+31
| export const RawKeySignerConfigSchema = z.object({ | ||
| type: z.literal(SignerType.RAW_KEY), | ||
| /** Direct private key (hex string with 0x prefix) */ | ||
| privateKey: ZHash.optional(), | ||
| /** Environment variable name containing the private key */ | ||
| privateKeyEnvVar: z.string().optional(), | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n typescript/sdk/src/signers/config.ts | head -50Repository: hyperlane-xyz/hyperlane-monorepo
Length of output: 1899
🏁 Script executed:
rg "RawKeySignerConfigSchema" typescript/sdk/src/ -A 2 -B 2Repository: hyperlane-xyz/hyperlane-monorepo
Length of output: 1653
🏁 Script executed:
rg "ChainMap" typescript/sdk/src/ --max-count 10Repository: hyperlane-xyz/hyperlane-monorepo
Length of output: 24710
🌐 Web query:
Zod refine and superRefine for conditional required fields validation
💡 Result:
refine vs superRefine (what matters for “conditionally required”)
refine(predicate, options): returns one boolean pass/fail, and (practically) attaches one issue (one path). Good for simple checks. (valentinprugnaud.dev)superRefine((data, ctx) => …): lets you add multiple issues, each with its ownpath, viactx.addIssue(...). Best for cross-field + conditional required logic. (valentinprugnaud.dev)
Also important: refine/superRefine run after the object has passed the base shape/type checks (so you may not see your conditional errors until other required fields are fixed). (github.com)
Pattern: “Field B is required when Field A has value X”
Make B optional in the shape, then enforce requirement in superRefine:
import { z } from "zod";
const schema = z
.object({
type: z.enum(["Private", "Business"]),
organisationName: z.string().optional(),
})
.superRefine((data, ctx) => {
if (data.type === "Business" && !data.organisationName?.trim()) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
path: ["organisationName"],
message: "Organisation name is required when type is Business",
});
}
});This is the standard Zod way to do conditional required fields (and lets you target the exact field via path). (valentinprugnaud.dev)
Pattern: “At least one of these fields is required” (and show errors on multiple fields)
Use superRefine and add an issue per field you want highlighted:
const schema = z
.object({
warehouses: z.array(z.string()).optional(),
clusters: z.array(z.string()).optional(),
})
.superRefine((data, ctx) => {
const w = data.warehouses?.length ?? 0;
const c = data.clusters?.length ?? 0;
if (w === 0 && c === 0) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
path: ["warehouses"],
message: "Select at least one warehouse or cluster",
});
ctx.addIssue({
code: z.ZodIssueCode.custom,
path: ["clusters"],
message: "Select at least one warehouse or cluster",
});
}
});Reason: refine({ path: [...] }) won’t reliably “broadcast” to multiple paths; superRefine is the recommended approach. (stackoverflow.com)
If you need conditional errors to appear even when other required fields fail
Because refinements run after base validation, you have a few common workarounds:
- Model the condition structurally with a
discriminatedUnion(often best for forms with a “mode/type” field). (github.com) - Move conditional logic into a nested object so it can validate independently of other top-level required fields. (github.com)
- (Less common) Use preprocessing / custom parsing, but it’s usually more complex than (1) or (2). (github.com)
Citations:
- 1: https://valentinprugnaud.dev/posts/zod-series/3/advanced-zod-designing-complex-validation-schemas?utm_source=openai
- 2: https://valentinprugnaud.dev/posts/zod-series/3/advanced-zod-designing-complex-validation-schemas?utm_source=openai
- 3: What is zod's approach to conditionally requiring fields? colinhacks/zod#938
- 4: https://valentinprugnaud.dev/posts/zod-series/3/advanced-zod-designing-complex-validation-schemas?utm_source=openai
- 5: https://stackoverflow.com/questions/79509075/zod-refine-not-displaying-errors-for-optional-array-fields-in-react-hook-form?utm_source=openai
- 6: How can I validate a field conditionally based on the value of another field in a zod form schema, ensuring all validations run simultaneously? colinhacks/zod#3268
- 7: What is zod's approach to conditionally requiring fields? colinhacks/zod#938
- 8: What is zod's approach to conditionally requiring fields? colinhacks/zod#938
RawKey schema allows empty configs; add validation to require at least one key source.
Both privateKey and privateKeyEnvVar are optional, so { type: 'rawKey' } passes validation but fails at runtime. Add a refinement to require at least one:
Enforce at-least-one requirement
-export const RawKeySignerConfigSchema = z.object({
- type: z.literal(SignerType.RAW_KEY),
- /** Direct private key (hex string with 0x prefix) */
- privateKey: ZHash.optional(),
- /** Environment variable name containing the private key */
- privateKeyEnvVar: z.string().optional(),
-});
+export const RawKeySignerConfigSchema = z
+ .object({
+ type: z.literal(SignerType.RAW_KEY),
+ /** Direct private key (hex string with 0x prefix) */
+ privateKey: ZHash.optional(),
+ /** Environment variable name containing the private key */
+ privateKeyEnvVar: z.string().optional(),
+ })
+ .refine(
+ (cfg) => !!cfg.privateKey || !!cfg.privateKeyEnvVar,
+ { message: 'Provide privateKey or privateKeyEnvVar' },
+ );Consider using superRefine if you need path-specific error targeting.
🤖 Prompt for AI Agents
In `@typescript/sdk/src/signers/config.ts` around lines 25 - 31,
RawKeySignerConfigSchema currently allows an empty config because both
privateKey and privateKeyEnvVar are optional; update RawKeySignerConfigSchema
(the z.object with type: z.literal(SignerType.RAW_KEY), privateKey,
privateKeyEnvVar) to add a zod refinement (use superRefine) that checks at least
one of privateKey or privateKeyEnvVar is present and emits a clear error path
(e.g., add errors for privateKeyEnvVar or privateKey) when neither is provided
so validation fails early instead of at runtime.
Comment on lines
+319
to
+376
| private static async fetchGCPSecret( | ||
| project: string, | ||
| secretName: string, | ||
| ): Promise<string> { | ||
| const { exec } = await import('child_process'); | ||
| const { promisify } = await import('util'); | ||
| const execAsync = promisify(exec); | ||
|
|
||
| const command = `gcloud secrets versions access latest --secret="${secretName}" --project="${project}"`; | ||
| logger.debug(`Fetching secret from GCP: ${project}/${secretName}`); | ||
|
|
||
| try { | ||
| const { stdout, stderr } = await execAsync(command, { | ||
| maxBuffer: 1024 * 1024, // 1MB buffer | ||
| }); | ||
|
|
||
| if (stderr) { | ||
| logger.warn(`gcloud stderr: ${stderr}`); | ||
| } | ||
|
|
||
| const secret = stdout.trim(); | ||
| if (!secret) { | ||
| throw new Error(`Secret ${secretName} is empty`); | ||
| } | ||
|
|
||
| return secret; | ||
| } catch (error: any) { | ||
| const errorMessage = error.stderr || error.message || String(error); | ||
|
|
||
| // Check for common gcloud errors and provide helpful messages | ||
| if (errorMessage.includes('command not found')) { | ||
| throw new Error( | ||
| 'gcloud CLI not found. Install the Google Cloud SDK: https://cloud.google.com/sdk/docs/install', | ||
| ); | ||
| } | ||
| if ( | ||
| errorMessage.includes('PERMISSION_DENIED') || | ||
| errorMessage.includes('does not have permission') | ||
| ) { | ||
| throw new Error( | ||
| `Permission denied accessing secret ${project}/${secretName}. ` + | ||
| `Ensure you have roles/secretmanager.secretAccessor permission.`, | ||
| ); | ||
| } | ||
| if (errorMessage.includes('NOT_FOUND')) { | ||
| throw new Error( | ||
| `Secret not found: ${project}/${secretName}. ` + | ||
| `Verify the project ID and secret name are correct.`, | ||
| ); | ||
| } | ||
| if (errorMessage.includes('Could not load the default credentials')) { | ||
| throw new Error( | ||
| 'GCP authentication required. Run: gcloud auth application-default login', | ||
| ); | ||
| } | ||
|
|
||
| throw new Error(`Failed to fetch GCP secret: ${errorMessage}`); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the actual file to confirm code and context
head -n 380 typescript/sdk/src/signers/SignerFactory.ts | tail -n 70Repository: hyperlane-xyz/hyperlane-monorepo
Length of output: 2438
🏁 Script executed:
#!/bin/bash
# Search for where SignerConfiguration and GCP secrets are used
rg -n "getSignerConfiguration|SignerConfiguration" typescript/sdk/src -g "*.ts" -A 2 -B 2Repository: hyperlane-xyz/hyperlane-monorepo
Length of output: 4355
🏁 Script executed:
#!/bin/bash
# Search for GCP secret references and gcloud invocations
rg -n "gcpSecret|GCP_SECRET|gcloud" typescript/sdk/src -g "*.ts" -B 3 -A 3Repository: hyperlane-xyz/hyperlane-monorepo
Length of output: 10336
Switch from exec to execFile to prevent shell injection when invoking gcloud.
The code currently interpolates project and secretName directly into a shell command string. This allows shell injection if either value contains special characters (e.g., secret$(cmd) or proj; malicious). Since this code fetches cryptographic keys—security-critical operations—it must avoid shell parsing entirely. Use execFile with an args array instead, which executes the gcloud binary directly without invoking a shell.
🔒 Safer gcloud invocation
- const { exec } = await import('child_process');
- const { promisify } = await import('util');
- const execAsync = promisify(exec);
-
- const command = `gcloud secrets versions access latest --secret="${secretName}" --project="${project}"`;
+ const { execFile } = await import('child_process');
+ const { promisify } = await import('util');
+ const execFileAsync = promisify(execFile);
@@
- const { stdout, stderr } = await execAsync(command, {
- maxBuffer: 1024 * 1024, // 1MB buffer
- });
+ const { stdout, stderr } = await execFileAsync(
+ 'gcloud',
+ [
+ 'secrets',
+ 'versions',
+ 'access',
+ 'latest',
+ `--secret=${secretName}`,
+ `--project=${project}`,
+ ],
+ { maxBuffer: 1024 * 1024 }, // 1MB buffer
+ );🤖 Prompt for AI Agents
In `@typescript/sdk/src/signers/SignerFactory.ts` around lines 319 - 376, The
fetchGCPSecret function currently builds a shell command and uses exec, which
permits shell injection; replace the dynamic command string and promisified exec
with promisified execFile (imported from 'child_process') and pass gcloud as the
executable with an args array like ['secrets', 'versions', 'access', 'latest',
'--secret', secretName, '--project', project] (preserve the maxBuffer option),
then consume stdout/stderr the same way and keep the existing error
parsing/rewriting logic so all the current permission/auth/not-found checks
remain intact; ensure you update the variable names (e.g., execAsync ->
execFileAsync) and maintain logger.debug showing project/secretName without
executing them in a shell.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7938 +/- ##
=======================================
Coverage 77.02% 77.02%
=======================================
Files 117 117
Lines 2651 2651
Branches 244 244
=======================================
Hits 2042 2042
Misses 593 593
Partials 16 16
🚀 New features to boost your workflow:
|
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
2 participants
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
Implement signer abstraction that allows CLI users to configure signers through registry files or URIs instead of passing private keys via
--key.Related PR: hyperlane-xyz/hyperlane-registry#1364
Motivation
Passing private keys via
--keycommand-line argument has security issues:This PR enables:
Implementation
SDK Changes (
typescript/sdk/src/signers/)config.ts - Signer type definitions with Zod schemas:
rawKey- Direct private key or env var referencegcpSecret- GCP Secret Manager referencefoundryKeystore- Foundry-compatible encrypted keystoreturnkey- Turnkey secure enclave signingSignerFactory.ts - Create ethers Signers from config:
gcloudCLI (no SDK dependency)ETH_PASSWORDenv var (Foundry standard)extractPrivateKey()for exporting keys to external toolsCLI Changes
context.ts - Load signer config from registry via
getSignerConfiguration()MultiProtocolSignerManager.ts - Resolve signers with fallback:
--keyargument fallbackregistry.ts - New
hyperlane registry signer-keycommand:Usage Examples
YAML Configuration Example
Testing
Documentation
typescript/cli/SIGNER_REGISTRY.md- Comprehensive guideChecklist
signer-keycommand for key extractionETH_PASSWORD)Summary by CodeRabbit
Release Notes
New Features
signer-keyCLI command to extract and retrieve keys and addresses from configured registriesDocumentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.