fix(signer): verify required env vars exist during config validation

[raushan728]

Signer validation only checked if env var names were non-empty strings —
it never verified the variables actually exist on the host. Operators
could pass config validate only to have the node crash at runtime.

---

[greptile-apps]

Greptile Summary

This PR closes a real operator footgun: signer validation previously only checked that env-var names were non-empty strings, never that the referenced variables were actually present on the host. The fix threads get_env_var_for_signer calls into all six validate_*_config functions and updates tests accordingly.

Key changes

• validate_memory_config, validate_turnkey_config, validate_privy_config, validate_vault_config, validate_aws_kms_config, and validate_fireblocks_config all now call get_env_var_for_signer for each env-var field, returning a ValidationError if any variable is absent.
• Updated test_validate_config_success and test_load_signers_config to set/remove the required TEST_PRIVATE_KEY env var around assertions.
• Added two new focused tests (test_validate_memory_config_missing_env_var, test_validate_memory_config_env_var_present) that cover the happy- and error-paths for the new check.
• Updated signer_validator.rs test_validate_with_result_warnings to set TEST_KEY before invoking validation.

Issues found

• validate_aws_kms_config checks only the two required fields (key_id_env, public_key_env) and skips region_env. If an operator provides a region_env that names a non-existent variable, validation still passes and the node crashes at runtime — the exact failure mode the PR aims to prevent.
• test_validate_config_success and test_load_signers_config both mutate TEST_PRIVATE_KEY without any synchronization. Rust runs tests in parallel by default; one test's remove_var can race the other's set_var, causing intermittent failures. The new tests wisely use unique, unlikely-to-collide names and ConfigMockBuilder — the same approach should be applied to the updated pre-existing tests.

Confidence Score: 3/5

• The core fix is correct and valuable, but an incomplete validation gap for the optional AWS KMS region_env and a test race condition should be addressed before merging.
• The approach is sound and the logic for all required env-var fields is correct. However, validate_aws_kms_config leaves the optional region_env unchecked — a scenario the PR is specifically designed to catch — and the two existing tests that were updated to set/remove TEST_PRIVATE_KEY are susceptible to parallel-test races, which can produce flaky CI failures.
• crates/lib/src/signer/config.rs — specifically validate_aws_kms_config (missing region_env check) and the test section (concurrent env-var mutation).

Important Files Changed

Filename	Overview
crates/lib/src/signer/config.rs	Adds get_env_var_for_signer calls to all six validate_*_config functions so missing env vars are caught at config-load time. Two gaps remain: AwsKmsSignerConfig::region_env (optional but validated at runtime) is not checked during validation, and the two updated pre-existing tests both mutate TEST_PRIVATE_KEY without isolation, making them prone to parallel-test races.
crates/lib/src/validator/signer_validator.rs	Adds ConfigMockBuilder setup and TEST_KEY env-var lifecycle to test_validate_with_result_warnings so it passes with the new env-var existence check. No logic changes to the validator itself; changes are test-only and straightforward.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[load_config / validate_signer_config] --> B{For each SignerConfig}
    B --> C[validate_individual_signer_config]
    C --> D{Signer type?}
    D -->|Memory| E[validate_memory_config]
    D -->|Turnkey| F[validate_turnkey_config]
    D -->|Privy| G[validate_privy_config]
    D -->|Vault| H[validate_vault_config]
    D -->|AwsKms| I[validate_aws_kms_config]
    D -->|Fireblocks| J[validate_fireblocks_config]

    E --> E1{private_key_env empty?}
    E1 -->|Yes| ERR[ValidationError]
    E1 -->|No| E2[get_env_var_for_signer ✅ NEW]
    E2 -->|Var missing| ERR

    I --> I1{key_id_env / public_key_env empty?}
    I1 -->|Yes| ERR
    I1 -->|No| I2[get_env_var_for_signer ✅ NEW]
    I2 -->|OK| I3{region_env Some?}
    I3 -->|Yes| I4[⚠️ NOT validated — runtime crash if missing]
    I3 -->|No| OK[✅ Validation passes]
    I2 -->|Var missing| ERR

    F --> F1[Check all 5 env vars empty?]
    F1 -->|Any empty| ERR
    F1 -->|None empty| F2[get_env_var_for_signer ✅ NEW x5]
    F2 -->|Any missing| ERR
    F2 --> OK

    ERR --> FAIL[Config load fails early ✅]
    OK --> PASS[Node starts safely ✅]

Loading

Comments Outside Diff (1)

1. crates/lib/src/signer/config.rs, line 481-493 (link)

   Optional region_env skipped during validation

   validate_aws_kms_config only checks key_id_env and public_key_env, but silently skips region_env. If an operator provides a non-empty region_env value that names a variable not set on the host, validation passes but build_aws_kms_signer will call get_env_var_for_signer on it at runtime and crash — exactly the failure mode this PR is meant to prevent.

   build_aws_kms_signer already handles the optional case with .transpose()?, so the same pattern should be applied during validation:

   fn validate_aws_kms_config(
       config: &AwsKmsSignerConfig,
       signer_name: &str,
   ) -> Result<(), KoraError> {
       let env_vars =
           [("key_id_env", &config.key_id_env), ("public_key_env", &config.public_key_env)];
   
       for (field_name, env_var) in env_vars {
           if env_var.is_empty() {
               return Err(KoraError::ValidationError(format!(
                   "AWS KMS signer '{signer_name}' must specify non-empty {field_name}"
               )));
           }
           get_env_var_for_signer(env_var, signer_name)?;
       }
   
       // Also validate the optional region_env if it is specified
       if let Some(region_env) = &config.region_env {
           if !region_env.is_empty() {
               get_env_var_for_signer(region_env, signer_name)?;
           }
       }
   
       Ok(())
   }

_{Last reviewed commit: f3694da}

[raushan728]

Fixed both race conditions — renamed TEST_PRIVATE_KEY to unique names
(KORA_VALIDATE_SUCCESS_KEY_99 and KORA_LOAD_CONFIG_KEY_99) in the two
pre-existing tests to prevent parallel test conflicts.

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

[github-actions]

✅ Fork external live tests passed.

fork-external-live-pass:401704180dbe361365ecf257768b9b2e9028c8e5
run: https://github.com/solana-foundation/kora/actions/runs/22861625595