payments-stripe: read STRIPE_API_KEY from env, drop api_key from Input - #38
Merged
mike-hanono merged 1 commit intoMay 25, 2026
Conversation
Stacks on #37 (which already moved the crate to offchain/tools/ and added tools.json + build.rs). This PR is the security fix #37 deliberately left out: every endpoint's Input struct had `pub api_key: String`, which flowed through the Nexus DAG to Sui as plaintext on every invocation. Adopts the env-var credential pattern from offchain/tools/memory-memwal: - stripe_client.rs: `from_env()` reads STRIPE_API_KEY once at startup, wraps in `zeroize::Zeroizing<String>` (heap-zeroed on drop), validates the prefix (`sk_test_` / `sk_live_` / `rk_test_` / `rk_live_`). Hand- written `Debug` prints `<redacted>` — never `#[derive(Debug)]` on the credential-bearing type. Shared HTTP client lives in a `OnceLock` so the connection pool survives across `invoke` calls. `#[cfg(test)] for_testing(base_url, bearer)` constructor bypasses env validation for mockito tests. - main.rs: explicit multi-thread tokio runtime; `env_logger` init before anything else; `--meta` short-circuit so CI's prepare step can pull /meta from an image with no env; `load_dotenv_if_present()` and `validate_credentials_at_startup()` before `bootstrap!`. `main` is the only `process::exit` site. Exit 1 if STRIPE_API_KEY is unset, empty, or has an unrecognized prefix. - All six endpoint files: dropped `pub api_key: String` from Input. Dropped `.with_auth(&input.api_key)` from invoke(). NexusTool::new() calls `StripeClient::from_env().unwrap_or_else(|e| { log::error!(...); panic!(...) })` — startup-only panic, mirrors memwal. `idempotency_key` stays on Input (per-call dedup data, not a credential — explicit whitelist in the auditor). - Tests: every `StripeClient::new(Some(&server.url()))` → `StripeClient::for_testing(&server.url(), "sk_test_FAKE")`. Every `api_key:` field dropped from `test_input()` and from inline Input literals. `get-balance` Input is now empty (`{}`) — only the STRIPE_API_KEY env var matters; the `_input` arg in invoke is unused. - Cargo.toml: added workspace deps `anyhow`, `dotenvy`, `env_logger`, `log`, `zeroize` for the env-var path. - README.md: rewrote the credential section to document the env var. Dropped `api_key` from every per-FQN Input table. Added an env-vars table. Threaded `<TOOL_FQN_VERSION>` into each FQN heading (matching the actual `concat!(..., env!("TOOL_FQN_VERSION"))` in source). - .env.example: documents STRIPE_API_KEY for local dev. In production Cloud Run mounts it via `secretKeyRef` from GCP Secret Manager, configured by the operator out-of-band — the deploy pipeline does NOT provision the upstream API key. tools.json's `environment` block is RUST_LOG only (C10). - Regenerated AUDIT.md is deferred — run the auditor agent against the merged tree, or invoke it after this branch lands. Markdown linter fix: aligned 4 skill reference files' compact table separators (`|---|` → `| --- |`) to satisfy MD060. These same files are rewritten by PR #31; the conflict at merge is a no-op since both branches use `| --- |`. Touched only to unblock `just pre-commit::md-lint` on this branch. Verified locally: - bash <audit.sh from #31's tip> off-chain payments-stripe → PASS static:input-credential → PASS static:debug-on-credentials → PASS static:stateful → PASS tools-json:shape / secret-in-env → PASS conform:output-enum / description / fqn-versioned (cargo:check fails — sandbox is missing pkg-config/libssl-dev for openssl-sys; orthogonal to this change. PR #37 author noted the same in their PR description; verification happens via CI on push.) - npx markdownlint-cli2 '**/*.md' '!.git/' → 0 errors across 26 files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mike-hanono
merged commit May 25, 2026
90cfdd7
into
chore/payments-stripe-offchain-conventions
7 checks passed
5 tasks
mike-hanono
pushed a commit
that referenced
this pull request
May 25, 2026
PR #38 added anyhow / dotenvy / env_logger / log / zeroize to payments-stripe's Cargo.toml but never refreshed the lockfile. CI builds with `cargo build --locked` which refuses to update Cargo.lock at build time, so every tool's build job failed (shared workspace lockfile). Ran `cargo update --workspace --manifest-path offchain/Cargo.toml`. Diff is +8/-2 lines on offchain/Cargo.lock — only payments-stripe's entry changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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
Stacks on #37. Removes the
pub api_key: Stringfield from all 6 Stripe endpoint Input structs and switches the tool to the env-var credential pattern already used bymemory-memwalandstorage-walrus.Why this is critical: tool inputs flow through the Nexus DAG on Sui as plaintext. Any field on
Inputis effectively published on-chain. The currentapi_keyfield meant every Stripe DAG invocation was about to write the secret key (sk_test_…orsk_live_…) to a public ledger forever.What changes
src/stripe_client.rs—from_env()readsSTRIPE_API_KEYonce at startup; wraps inzeroize::Zeroizing<String>(heap-zeroed on drop); validates prefix (sk_test_/sk_live_/rk_test_/rk_live_) and length. Hand-writtenDebugprints<redacted>— no#[derive(Debug)]on the credential-bearing type. Shared HTTP client lives in aOnceLockso the connection pool survives across calls.#[cfg(test)] for_testing(base_url, bearer)constructor bypasses env validation for mockito tests.src/main.rs— Explicit multi-thread tokio runtime;env_loggerinit before anything else;--metashort-circuit so CI'spreparestep can pull/metafrom an image with no env mounted;load_dotenv_if_present()andvalidate_credentials_at_startup()beforebootstrap!.mainis the onlyprocess::exitsite. Exit 1 ifSTRIPE_API_KEYis unset, empty, or has an unrecognized prefix.All 6 endpoint files — Dropped
pub api_key: Stringfrom Input. Dropped.with_auth(&input.api_key)frominvoke().NexusTool::new()callsStripeClient::from_env().unwrap_or_else(|e| { log::error!(...); panic!(...) })— startup-only panic, mirrors memwal.idempotency_keystays on Input (per-call dedup data, not a credential — explicit whitelist in the auditor).Tests — Every
StripeClient::new(Some(&server.url()))→StripeClient::for_testing(&server.url(), "sk_test_FAKE").api_key:field dropped from everytest_input()and inline Input literal.get-balanceInput is now{}(no fields needed).Cargo.toml— Added workspace depsanyhow,dotenvy,env_logger,log,zeroize.README.md— Rewrote credential section to document the env var. Droppedapi_keyfrom every per-FQN Input table. Added an env-vars table. Threaded<TOOL_FQN_VERSION>into each FQN heading (matching what's actually in source viaconcat!(..., env!("TOOL_FQN_VERSION")))..env.example— DocumentsSTRIPE_API_KEYfor local dev. In production Cloud Run mounts it viasecretKeyReffrom GCP Secret Manager, configured by the operator out-of-band — the deploy pipeline does NOT provision the upstream API key.tools.json'senvironmentblock isRUST_LOGonly (C10 conformance).Skill markdown lints — 4 files in
.claude/skills/nexus-tool-builder/reference/had compact table separators (|---|) rewritten to| --- |to satisfy MD060 so this branch's pre-commit md-lint passes. These same files are rewritten by Add nexus-tool-builder skill + nexus-tool-auditor agent #31; the conflict at merge is a no-op since both branches use| --- |.Out of scope
nexus-tool-auditoragent against the merged tree, or invoke it after this branch lands. The version currently in the crate is stale (it documents the pre-fix Input shape).Test plan
api_keyno longer appears on anyInputstruct.cargo +stable check --package payments-stripe(blocked locally — sandbox missingpkg-config/libssl-devforopenssl-sys).cargo +stable clippy --package payments-stripe -- -D warnings.cargo +stable test --package payments-stripe(mockito tests withfor_testing()constructor).STRIPE_API_KEY=sk_test_… cargo run --release --package payments-stripethencurl /create-payment-intent/healthandcurl /create-payment-intent/meta.Verification done locally
main:static:input-credential(C1 — the central check this PR fixes)static:debug-on-credentials(C8)static:stateful(C9)tools-json:shape/tools-json:secret-in-env(C10, H7)conform:output-enum/description/fqn-versionednpx markdownlint-cli2 '**/*.md' '!.git/'→ 0 errors across 26 files.🤖 Generated with Claude Code