feat: validate required environment variables at service startup (fail fast) - #98
Open
temisan0x wants to merge 2 commits into
Open
feat: validate required environment variables at service startup (fail fast)#98temisan0x wants to merge 2 commits into
temisan0x wants to merge 2 commits into
Conversation
Wires validateEnvOrExit into every service entrypoint (orchestrator, registry, and all five agents), replacing ad-hoc single-var checks with aggregated fail-fast validation that reports every missing/ malformed key at once instead of one at a time. - orchestrator: REGISTRY_URL, ORCHESTRATOR_SECRET_KEY, ANTHROPIC_API_KEY (waived when LLM_PROVIDER=mock), AGENT_VAULT_CONTRACT_ID (optional) - registry: no required keys currently, validated for consistency - stellar-oracle, web-intel, web-intel-v2, analysis, reporter: REGISTRY_URL + service-specific wallet secret key - web-intel: ANTHROPIC_API_KEY marked optional (summarisation only) Adds packages/common/src/env.test.ts covering missing/present/ whitespace-only values, aggregated multi-key errors, optional keys, the LLM_PROVIDER=mock exception, and malformed shape values for url/number/stellarSecret/stellarContract types. Closes clevercon-protocol#88
📝 WalkthroughWalkthroughA shared environment validation helper and tests are added to ChangesEnvironment validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ServiceServer
participant validateEnvOrExit
participant requireEnv
participant Process
ServiceServer->>validateEnvOrExit: validate service environment
validateEnvOrExit->>requireEnv: check required and optional variables
requireEnv-->>validateEnvOrExit: return trimmed values or validation issues
validateEnvOrExit->>Process: exit with status 1 on invalid configuration
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/common/src/env.ts`:
- Around line 76-79: Update the return type of requireEnv so keys marked
optional in EnvSpec are exposed as string | undefined while required keys remain
string, matching the function’s omission behavior. Preserve the existing runtime
result and add a regression test verifying an absent optional key is typed as
possibly undefined.
- Around line 37-38: Replace STELLAR_SECRET_RE and STELLAR_CONTRACT_RE in
packages/common/src/env.ts lines 37-38 with checksum-aware Stellar validation,
reusing an existing shared Stellar parser or validator where available rather
than checking textual shape only. In packages/common/src/env.test.ts lines
144-166, add cases that mutate one character of VALID_SECRET and VALID_CONTRACT
and assert both checksum-invalid values fail validation.
In `@packages/orchestrator/src/server.ts`:
- Around line 63-70: Retain the normalized object returned by validateEnvOrExit
and use it for startup configuration instead of raw process.env values: update
packages/orchestrator/src/server.ts (lines 63-70) for required orchestrator
settings; packages/agents/analysis/src/server.ts (lines 12-17),
packages/agents/reporter/src/server.ts (lines 12-17),
packages/agents/stellar-oracle/src/server.ts (lines 20-25), and
packages/agents/web-intel-v2/src/server.ts (lines 12-17) for the validated
secret and registry URL; and packages/agents/web-intel/src/server.ts (lines
14-20) for the validated secret, registry URL, and present optional key.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4b61a0a7-328e-456f-886a-a298369920d8
📒 Files selected for processing (10)
packages/agents/analysis/src/server.tspackages/agents/reporter/src/server.tspackages/agents/stellar-oracle/src/server.tspackages/agents/web-intel-v2/src/server.tspackages/agents/web-intel/src/server.tspackages/common/src/env.test.tspackages/common/src/env.tspackages/common/src/index.tspackages/orchestrator/src/server.tspackages/registry/src/server.ts
Comment on lines
+37
to
+38
| const STELLAR_SECRET_RE = /^S[A-Z2-7]{55}$/; | ||
| const STELLAR_CONTRACT_RE = /^C[A-Z2-7]{55}$/; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Validate Stellar checksums, not only the textual shape.
The regexes accept any 56-character base32 value with the right prefix, including values whose final character has been changed and whose Stellar checksum is invalid. Those malformed credentials pass fail-fast validation and fail later when used.
packages/common/src/env.ts#L37-L38: replace shape-only checks with checksum-aware Stellar secret and contract validation.packages/common/src/env.test.ts#L144-L166: add checksum-invalid cases by mutating one character ofVALID_SECRETandVALID_CONTRACT.
#!/bin/bash
# Check whether a checksum-aware Stellar parser already exists in shared dependencies.
fd -a 'package.json' packages -x sh -c '
printf "\n--- %s ---\n" "$1"
jq -r ".dependencies // {} | keys[]?" "$1" | grep -E "`@stellar`|stellar" || true
' sh {}
rg -n -C 2 --glob '*.ts' '\b(StrKey|Keypair)\.(fromSecret|fromPublicKey|isValid[A-Za-z]+)' packages📍 Affects 2 files
packages/common/src/env.ts#L37-L38(this comment)packages/common/src/env.test.ts#L144-L166
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/common/src/env.ts` around lines 37 - 38, Replace STELLAR_SECRET_RE
and STELLAR_CONTRACT_RE in packages/common/src/env.ts lines 37-38 with
checksum-aware Stellar validation, reusing an existing shared Stellar parser or
validator where available rather than checking textual shape only. In
packages/common/src/env.test.ts lines 144-166, add cases that mutate one
character of VALID_SECRET and VALID_CONTRACT and assert both checksum-invalid
values fail validation.
Comment on lines
+76
to
+79
| export function requireEnv<S extends EnvSpec>( | ||
| spec: S, | ||
| source: NodeJS.ProcessEnv = process.env | ||
| ): { [K in keyof S]: string } { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect consumers that may rely on optional returned values being strings.
rg -n -C 3 --glob '*.ts' '\brequireEnv\s*\(' packagesRepository: clevercon-protocol/clevercon
Length of output: 8972
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file shape first, then inspect the relevant section.
ast-grep outline packages/common/src/env.ts --view expanded || true
printf '\n--- env.ts excerpt ---\n'
sed -n '1,220p' packages/common/src/env.ts
printf '\n--- optional-key usages ---\n'
rg -n -C 2 --glob '*.ts' 'optional:\s*true|optional:\s*source' packages/common/srcRepository: clevercon-protocol/clevercon
Length of output: 6539
Make optional env keys optional in the return type
requireEnv omits missing optional keys from result, but the signature still exposes every key as string. That lets requireEnv({ MY_OPT: { optional: true } }).MY_OPT.trim() type-check even when MY_OPT is absent. Return string | undefined for optional keys (or model the result as Partial) and add a regression test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/common/src/env.ts` around lines 76 - 79, Update the return type of
requireEnv so keys marked optional in EnvSpec are exposed as string | undefined
while required keys remain string, matching the function’s omission behavior.
Preserve the existing runtime result and add a regression test verifying an
absent optional key is typed as possibly undefined.
Comment on lines
+63
to
+70
| if (!process.env.VITEST) { | ||
| validateEnvOrExit('orchestrator', { | ||
| REGISTRY_URL: { type: 'url' }, | ||
| ORCHESTRATOR_SECRET_KEY: { type: 'stellarSecret' }, | ||
| ANTHROPIC_API_KEY: { optional: process.env.LLM_PROVIDER === 'mock' }, | ||
| AGENT_VAULT_CONTRACT_ID: { optional: true, description: 'vault calls become safe no-ops when unset' }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Consume the normalized values returned by validation.
Each entrypoint discards validateEnvOrExit’s trimmed result and continues reading raw process.env. A whitespace-padded secret therefore validates successfully but remains padded when passed to downstream startup configuration.
packages/orchestrator/src/server.ts#L63-L70: retain the validated object and source required orchestrator configuration from it.packages/agents/analysis/src/server.ts#L12-L17: use the validated secret and registry URL instead of raw environment reads.packages/agents/reporter/src/server.ts#L12-L17: use the validated secret and registry URL instead of raw environment reads.packages/agents/stellar-oracle/src/server.ts#L20-L25: use the validated secret and registry URL instead of raw environment reads.packages/agents/web-intel-v2/src/server.ts#L12-L17: use the validated secret and registry URL instead of raw environment reads.packages/agents/web-intel/src/server.ts#L14-L20: use the validated secret, registry URL, and present optional key instead of raw environment reads.
📍 Affects 6 files
packages/orchestrator/src/server.ts#L63-L70(this comment)packages/agents/analysis/src/server.ts#L12-L17packages/agents/reporter/src/server.ts#L12-L17packages/agents/stellar-oracle/src/server.ts#L20-L25packages/agents/web-intel-v2/src/server.ts#L12-L17packages/agents/web-intel/src/server.ts#L14-L20
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/orchestrator/src/server.ts` around lines 63 - 70, Retain the
normalized object returned by validateEnvOrExit and use it for startup
configuration instead of raw process.env values: update
packages/orchestrator/src/server.ts (lines 63-70) for required orchestrator
settings; packages/agents/analysis/src/server.ts (lines 12-17),
packages/agents/reporter/src/server.ts (lines 12-17),
packages/agents/stellar-oracle/src/server.ts (lines 20-25), and
packages/agents/web-intel-v2/src/server.ts (lines 12-17) for the validated
secret and registry URL; and packages/agents/web-intel/src/server.ts (lines
14-20) for the validated secret, registry URL, and present optional key.
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
1 participant
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
Adds a fail-fast environment variable validation helper and wires it into
every service's entrypoint, so a misconfigured deploy exits immediately
with a single aggregated error listing everything wrong — instead of
failing late, deep inside a task, with a confusing runtime error.
Closes #88
Changes
packages/common/src/env.ts— newrequireEnv(spec, source?)andvalidateEnvOrExit(serviceName, spec, source?)helpers. Collects allviolations before throwing/exiting (not one at a time). Supports typed
shape checks:
string,url,number,stellarSecret,stellarContract.Empty/whitespace-only values are treated as missing.
packages/common/src/index.ts— exports the new module.server.ts(orchestrator, registry, stellar-oracle, web-intel,web-intel-v2, analysis, reporter) now calls
validateEnvOrExitbefore anyprocess.env.Xis read into a const, replacing the old ad-hocsingle-key
if (!SECRET_KEY) { ... process.exit(1) }checks.LLM_PROVIDER=mockcorrectly waivesANTHROPIC_API_KEYon theorchestrator.
AGENT_VAULT_CONTRACT_IDstays optional (vault becomes a safe no-op whenunset, per existing behavior).
web-intel'sANTHROPIC_API_KEYstays optional (used only for optionalClaude summarisation).
.env.example— already in sync,no changes needed there.
packages/common/src/env.test.ts— Vitest coverage for missing/present/whitespace-only values, aggregated multi-missing errors, optional keys,
the mock exception (both directions), and malformed shapes for all 4
typed checks.
server.tsvalidation calls are skipped underprocess.env.VITESTso the existing test suite is unaffected.
Testing
npm test— 131 passed, 6 skipped (pre-existing integration test skip,unrelated to this change)
npm run typecheck— passes on all 8 packagesSummary by CodeRabbit
New Features
Bug Fixes