feat(signing): generate_signing_keypair() programmatic API (closes #217)#243
Merged
feat(signing): generate_signing_keypair() programmatic API (closes #217)#243
Conversation
Round-8 webhooks-9421 agent flagged that adcp-keygen on PATH isn't
reliable from CI / subprocess contexts where the venv's bin dir
isn't on PATH (spawning .venv/bin/python directly raised
FileNotFoundError). Callers worked around with
Path(sys.executable).parent / "adcp-keygen" — fine, but shell-out is
the wrong shape for tests and provisioning code already running in
the Python process.
Add generate_signing_keypair(*, alg, kid, purpose, passphrase) →
(pem_bytes, public_jwk). Single entry point that dispatches to the
existing generate_ed25519 / generate_es256 helpers after resolving
the default kid (adcp-{alg}-YYYYMMDD). main() in the CLI now
delegates to the same helper — zero duplication between CLI and
programmatic paths.
Fails loud on unsupported alg / purpose. Re-exported from
adcp.signing.
- 18 unit tests: PEM loads via cryptography, JWK public matches PEM
public via DER round-trip, default + explicit purpose, invalid
inputs raise, encrypted PEM passphrase round-trip, JWK shape per
alg (no private scalar leaks), ed25519 + es256 sign+verify against
the produced JWK, WebhookSender.from_pem accepts the PEM, kid
default format, top-level re-export.
- Docs quickstart block in src/adcp/signing/__init__.py showing
programmatic + CLI equivalence.
Existing keygen tests keep passing — the CLI is now a thin wrapper
around the same spine.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both reviewers converged on two findings — both fixed:
Security M1 + code nit: Doc example wrote PEM via Path.write_bytes
which inherits the process umask (often 0644 = world-readable).
Rewrote both the keygen module docstring and the signing/__init__.py
quickstart to model the O_EXCL | 0o600 pattern the CLI already uses.
Copy-paste readers now ship the secure default.
Security M2 + code should-fix: default kid was adcp-{alg}-YYYYMMDD
— date-stable per UTC day. Two rotations in 24h collided on the
same kid, producing silent verification failures (verifiers cache
the first JWK, reject signatures from the second as
REQUEST_SIGNATURE_INVALID). Added a 4-char random hex suffix
(secrets.token_hex(2)) so same-day calls diverge. Docstring now
warns rotation callers MUST supply their own kid and the default
is opaque — not a contract downstream tooling may parse.
Code should-fix: test_cli_main_still_calls_helper was a tautology
(asserting callable(main) + callable(public)). Replaced with a
monkeypatch spy that asserts the CLI called generate_signing_keypair
with the expected kwargs. Catches accidental re-inlining.
Code should-fix: default_kid format test now asserts only
"non-empty string" plus adds a same-day collision-resistance test.
Doesn't lock the implementation to a particular format string.
Code should-fix: encrypted PEM round-trip test now parametrized
over ed25519 + es256 so a regression in the es256 encryption path
can't escape.
Security L2: passphrase lifecycle note added to the API docstring —
CPython can't zero bytes; long-lived credential callers should
source from a secret manager rather than holding a literal.
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
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
Closes #217 — round-8 polish completing the trio (after #242's skills bundle).
Round-8 webhooks-9421 agent flagged `adcp-keygen` on PATH isn't reliable from CI / subprocess contexts where the venv's bin dir isn't on `PATH`. Shell-out is the wrong shape for tests and provisioning code already in the Python process.
Adds `generate_signing_keypair(*, alg, kid, purpose, passphrase)` → `(pem_bytes, public_jwk)` as the single programmatic entry point. Re-exported from `adcp.signing`.
```python
from adcp.signing import generate_signing_keypair
pem, public_jwk = generate_signing_keypair(
alg="ed25519", purpose="webhook-signing"
)
Path("key.pem").write_bytes(pem)
publish_to_jwks_uri(public_jwk)
```
Zero duplication
CLI `main()` now delegates to the same helper. The CLI is a thin wrapper around a shared spine — a PEM minted via the helper is indistinguishable from one the CLI wrote, and regressions in either surface show up in both. Existing keygen tests keep passing unchanged.
Fails loud
Unsupported `alg` or `purpose` raises `ValueError` at the boundary. Silent fallback would mint a key with the wrong `adcp_use` claim — which every conformant verifier rejects, silently at first delivery.
Test plan
🤖 Generated with Claude Code