feat(CPL-284): add sig-derived usage API key endpoint for ChainSecured mode#326
Conversation
…d mode
Adds POST /core/v1/add_usage_api_key_with_signature, the ChainSecured
counterpart to /add_usage_api_key. Mirrors create_wallet_with_signature:
the user proves wallet ownership with an EIP-191 personal_sign over a
SIWE-style message (Address / Chain ID / Issued At, ±300 s skew); the
server mints a usage-key wallet via DStack and returns the secret
(base64) plus wallet address + derivation path. The client follows up
on-chain with registerWalletDerivation and setUsageApiKey signed by
their admin wallet — only the admin wallet of a ChainSecured account
can call setUsageApiKey (see AppStorage.accountExistsAndIsMutable).
Refactors the SIWE primitives now used by both ChainSecured endpoints:
- Extract verify_chainsecured_siwe (was inline in
create_wallet_with_signature) and rename parse_create_wallet_siwe ->
parse_chainsecured_siwe (and its struct).
- Reorder cheap rejects (length cap, parse, chain ID, timestamp window)
before the expensive ECDSA recovery so attacker traffic with stale
or wrong-network messages is dropped without doing crypto.
- Cap message length at 4 KiB.
- Reject duplicate Address / Chain ID / Issued At lines (last-wins
parsing previously could disagree with what the wallet UI showed).
Also updates the SIWE_TIMESTAMP_SKEW_SECONDS doc comment to describe
both consumers and reflect that returned bytes are equivalent to a
fresh keypair until the admin attaches them on-chain.
Regenerates k6/litApiServer.ts via:
cd lit-api-server && cargo run --bin openapi_spec > spec.json
npx @grafana/openapi-to-k6 spec.json ./k6
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds the new ChainSecured-mode endpoint to docs/management/api_direct.mdx under the existing "ChainSecured-mode HTTP endpoints" section, alongside create_wallet_with_signature and convert_to_chain_secured_account. Notes the two on-chain follow-up calls (registerWalletDerivation + setUsageApiKey) the admin wallet must run, the SIWE-lite request shape shared with create_wallet_with_signature, and the same ±5 min replay window. Updates the section preamble from "Two HTTP endpoints" to "Three HTTP endpoints". Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a new ChainSecured endpoint to mint usage API keys via an EIP-191 personal_sign (SIWE-lite) signature, reusing a hardened shared SIWE verification helper.
Changes:
- Added
POST /core/v1/add_usage_api_key_with_signature(no API-key guard) plus request/response models. - Extracted and reused
verify_chainsecured_siwe+ renamed SIWE parsing toparse_chainsecured_siwe, adding message hardening (size cap, duplicate-line rejection, cheaper rejects before ECDSA). - Regenerated the k6 TypeScript client to include the new endpoint and types.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lit-api-server/src/core/v1/models/response.rs | Adds response model for signature-derived usage API key minting. |
| lit-api-server/src/core/v1/models/request.rs | Adds request model for signature-derived usage API key minting. |
| lit-api-server/src/core/v1/endpoints/mod.rs | Registers the new endpoint in the v1 route/OpenAPI list. |
| lit-api-server/src/core/v1/endpoints/account_management.rs | Exposes the Rocket endpoint handler for add_usage_api_key_with_signature. |
| lit-api-server/src/core/account_management.rs | Implements the new core handler and shared ChainSecured SIWE verification/parsing hardening. |
Comments suppressed due to low confidence (1)
lit-api-server/src/core/account_management.rs:432
convert_to_chain_secured_accountnow reusesparse_chainsecured_siwe, but it still duplicates the rest of the SIWE verification logic (signature recovery, chain-id check, timestamp window) instead of calling the new sharedverify_chainsecured_siwe. This increases the chance of future divergence (e.g., message-length cap / reject ordering) between ChainSecured SIWE consumers. Consider usingverify_chainsecured_siwe(&req.message, &req.signature)here and then comparing the returned signer toclaimed_address.
let parsed = parse_chainsecured_siwe(&req.message)?;
if parsed.address != claimed_address {
return Err(ApiStatus::bad_request(
anyhow::anyhow!("Signed Address does not match new_admin_wallet_address"),
"Signed Address does not match new_admin_wallet_address",
));
}
let signer = recover_eip191_signer(&req.message, &req.signature)?;
if signer != claimed_address {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses Copilot review on PR #326. After CPL-284 introduced verify_chainsecured_siwe with new hardening (4 KiB length cap, cheap chain-id/timestamp checks before ECDSA recovery), convert_to_chain_secured_account was still inlining its own SIWE verification path and missing those protections. Replace the inline parse + recover + chain-id + timestamp block with a single call to verify_chainsecured_siwe, then check the recovered signer against the request body's claimed_address. The helper already enforces signer == parsed.address, so the transitive check (parsed.address == claimed_address) is preserved. All three ChainSecured-mode endpoints now share the same SIWE verification path, eliminating the divergence-risk Copilot flagged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Addresses an inline Copilot review finding on PR #326. The check `(now - parsed.issued_at).abs() > SIWE_TIMESTAMP_SKEW_SECONDS` could be bypassed for `Issued At: -9223372036854775808` (i.e., i64::MIN): - now - i64::MIN overflows i64. Release builds wrap to a value near i64::MIN. - i64::abs() on a value at/near i64::MIN returns i64::MIN itself (the positive 2^63 doesn't fit in i64). - Comparing i64::MIN against 300 is false, so the check passes and the skew window is bypassed. Compute the delta in i128 instead. Two i64 subtractions always fit in i128 without overflow, and i128::abs is safe for any value an i64 subtraction can produce. Net behavior is unchanged for legitimate (in-range) timestamps; pathological inputs are now correctly rejected by the abs() comparison instead of silently passing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot review addressedTwo findings, both real and now fixed. 1. (Inline, line 340) i64 overflow in SIWE timestamp window check
Confirmed real bypass. Fixed in e49b7280 by computing the delta in let delta = (now as i128) - (parsed.issued_at as i128);
if delta.abs() > SIWE_TIMESTAMP_SKEW_SECONDS as i128 {
return Err(...);
}Two 2. (Suppressed low-confidence in review body)
|
…pose tags CPL-285 (#327) shipped to next while this PR was open. It introduced two overlapping changes: - SIWE-lite domain separation via a `Purpose:` line + per-flow `SIWE_PURPOSE_*` constants (cross-flow replay prevention) — the same follow-up I flagged on this PR. - `encode_api_key_from_secret()` defense-in-depth that rejects base64-encoded secrets shaped like a precomputed account hash, preventing a confused-deputy collision with `usage_api_key_to_hash`. Resolution merges CPL-285's primitives with CPL-284's hardening into a single shared helper: - Drop my CPL-284 helper rename (verify_chainsecured_siwe / parse_chainsecured_siwe / ParsedChainsecuredSiwe). Use CPL-285's verify_siwe_signature / parse_create_wallet_siwe / ParsedCreateWalletSiwe names so all SIWE-using flows route through one symbol set. - Fold CPL-284's hardening into verify_siwe_signature: * 4 KiB MAX_SIWE_MESSAGE_LEN length cap (front-loaded reject) * Cheap rejects (parse, purpose, chain ID, timestamp) before ECDSA recovery so attacker traffic with stale or wrong-purpose messages is dropped without doing crypto * i128 timestamp delta to prevent the i64::MIN abs() bypass * Duplicate-line rejection (now also covers `Purpose:`) - Refactor create_wallet_with_signature, convert_to_chain_secured_account, and add_usage_api_key_with_signature to ALL call verify_siwe_signature (CPL-285 left create_wallet/convert inlined; this PR's Copilot review flagged the convert duplication, and the same critique applies to the create_wallet path — fixing both here matches the consistent pattern). - Add SIWE_PURPOSE_ADD_USAGE_API_KEY = "lit-add-usage-api-key-v1" for the new endpoint and pin it in the handler. - Use encode_api_key_from_secret(&secret)? instead of base64_light::base64_encode_bytes(&secret) so the new endpoint inherits CPL-285's confused-deputy guard. Docs updated: documents the `Purpose:` line as part of the shared SIWE envelope (was previously a CPL-285 doc gap), adds the per-endpoint `Purpose:` mapping table, and updates each curl example. Verified locally: - cargo fmt --all -- --check clean - cargo clippy --all-features -- -D warnings clean - cargo test --all-features (221 passed, 0 failed) - Regenerated k6 client matches OpenAPI spec byte-for-byte Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Adds
POST /core/v1/add_usage_api_key_with_signature, the ChainSecured counterpart to/add_usage_api_key. Mirrorscreate_wallet_with_signature(CPL-281): the user proves wallet ownership with an EIP-191personal_signover a SIWE-style message; the server mints a usage-key wallet via DStack and returns the secret (base64) plus wallet address + derivation path. The client follows up on-chain withregisterWalletDerivationandsetUsageApiKeysigned by their admin wallet — only the admin wallet of a ChainSecured account can callsetUsageApiKey(seeAppStorage.accountExistsAndIsMutable).Endpoint contract:
{ message, signature }— same SIWE-lite shape (Address:/Chain ID:/Issued At:, ±300 s skew) ascreate_wallet_with_signature{ usage_api_key, wallet_address, derivation_path }BilledManagementApiKeyguard — ChainSecured users have no master API key; the wallet signature IS the auth (matches sibling pattern)Helper hardening (applied across both ChainSecured endpoints):
verify_chainsecured_siwefrom inlinecreate_wallet_with_signatureso both endpoints share the same SIWE verification pathparse_create_wallet_siwe→parse_chainsecured_siwe(and the parsed struct) to reflect dual usageMAX_SIWE_MESSAGE_LEN)Address:/Chain ID:/Issued At:lines (last-wins parsing previously could disagree with what the wallet UI showed the user)SIWE_TIMESTAMP_SKEW_SECONDSdoc comment to describe both consumers and clarify the threat model: returned secret bytes are equivalent to a fresh keypair until the admin attaches them on-chaink6 client: regenerated via
cargo run --bin openapi_spec | npx @grafana/openapi-to-k6 …. NewaddUsageApiKeyWithSignaturemethod + types appear ink6/litApiServer.ts.Docs:
docs/management/api_direct.mdxupdated to document the new endpoint alongsidecreate_wallet_with_signatureandconvert_to_chain_secured_account(matches the pattern set by CPL-279).Test Coverage
No unit tests added for the SIWE primitives in this PR. Existing precedent (CPL-281's
create_wallet_with_signature,parse_create_wallet_siwe,recover_eip191_signer) has 0% direct unit coverage; this PR maintains that stance rather than introducing a new pattern.AI-assessed coverage on new branches: 0% (5 new code paths: length cap reject, 3 duplicate-line rejects, happy-path mint). Test stubs available for future hardening — see follow-up notes below. Coverage gap accepted in
/review.cargo test --all-featuresruns 209 existing tests, 0 failures (pre-existing baseline unchanged by this PR).Pre-Landing Review
Ran
/reviewwith full specialist coverage: testing + maintainability + security + api-contract specialists, Codex adversarial challenge, Claude adversarial subagent. 0 critical findings · 6 informational, all addressed.Auto-fixed: Stale doc comment above
SIWE_TIMESTAMP_SKEW_SECONDS.User-approved fixes applied (3 — all on the shared helper):
Skipped → flagged as follow-up tickets (see below):
PR Quality Score: 8.5/10 logged.
Adversarial Review
Codex flagged some items as
[P1](public unbilled key-minting oracle, non-idempotency on connection drop, two-step commit externalized). De-rated to follow-up severity because the threat model is no worse than the existingcreate_wallet_with_signature: returned bytes are unattached on-chain and equivalent to a freshLocalWallet::random()until the admin wallet callssetUsageApiKey. The "extra unregistered PKP — compute cost only" model continues to hold; the doc comment now says so accurately.TODOS
No items in
TODOS.mdmap to this PR's scope.Follow-up tickets to file
create_wallet_with_signatureandadd_usage_api_key_with_signatureare unauthenticated and trigger DStack PKP minting per call. No per-IP / per-signer throttle.Lower-priority follow-ups: precompute
usage_api_key_hashin response (saves clients a base64-decode + keccak); inline#[cfg(test)]forparse_chainsecured_siwe+recover_eip191_signerround-trip (test stubs ready).CI gates verified locally
cargo fmt --all -- --checkcleancargo clippy --all-features -- -D warningscleancargo test --all-features(209 passed, 0 failed)k6/litApiServer.tsmatches the freshly-emitted OpenAPI spec byte-for-byte (k6-client-check will pass)🤖 Generated with Claude Code