feat: add flowspec doctor and GPG signing#1239
Conversation
Add doctor health-check command and GPG agent signing module with CLI wiring, tests, and documentation. All Copilot review findings from PRs #1236, #1237, and #1238 are resolved. Doctor checks: CLI version, Python version, tool availability, workflow config, agent file naming (flow.*.agent.md), constitution. GPG signing: key generation via --status-fd, fingerprint validation (40-char hex), secret key verification by fingerprint, git config, key rotation, and keyring storage. Closes #1221 #1222 #1223 #1224 #1225 #1226 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: jason poley <jason.poley@gmail.com>
🟢 Security Scan Results (Advisory)
No critical or high severity issues found. How to remediate# View detailed scan results
gh run download 24012692520 -n security-scan-results-1c47243ad396b742eb3d6048d845d10699e3a7ca
# Triage findings with AI assistance
specify security triage
# Generate fix suggestions
specify security fix
# Apply fixes and re-scan
flowspec security scanSee the Security tab for detailed findings. |
There was a problem hiding this comment.
Pull request overview
Adds two new end-user features to the Flowspec CLI: a flowspec doctor environment health-check command and a flowspec gpg subcommand suite for agent GPG key management + git commit signing, including tests and a user guide.
Changes:
- Introduce
flowspec doctordiagnostics (version/Python/tooling/workflow config/agent naming/constitution) with Rich table output. - Add GPG signing core (
signing.py) and CLI wiring (gpg_cli.py,__init__.py) for setup/status/rotate workflows. - Add initial test coverage for both modules and a new documentation guide for GPG signing.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/flowspec_cli/doctor.py |
Implements the doctor checks and Rich rendering utilities. |
tests/test_doctor.py |
Adds unit tests for the doctor checks, including no-network behavior. |
src/flowspec_cli/signing.py |
Implements GPG key generation/storage and git signing configuration helpers. |
src/flowspec_cli/gpg_cli.py |
Adds Typer-based flowspec gpg CLI commands (setup/status/rotate). |
tests/test_signing.py |
Adds tests for signing/key management and command execution error handling. |
src/flowspec_cli/__init__.py |
Registers doctor command and gpg sub-app in the main CLI. |
docs/guides/gpg-signing.md |
Documents setup, status, rotation, and verification workflows for agent signing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def test_configure_git_signing_success( | ||
| self, mock_keyring, mock_git_commands, mock_gpg_commands, tmp_path | ||
| ): | ||
| """Test successful git configuration.""" | ||
| mock_keyring.get_password.return_value = "ABCD1234" | ||
| mock_gpg_commands.return_value = ( | ||
| "fpr:::::::::ABCD1234:\n", | ||
| "", | ||
| 0, | ||
| ) # secret key present, fingerprint matches | ||
| mock_git_commands.side_effect = [ | ||
| ("", "", 0), # rev-parse (check if git repo) | ||
| ("", "", 0), # config user.signingkey | ||
| ("", "", 0), # config commit.gpgsign | ||
| ] | ||
|
|
||
| configure_git_signing(project_root=tmp_path) | ||
|
|
||
| # Verify git commands were called correctly | ||
| assert mock_git_commands.call_count == 3 | ||
| assert mock_git_commands.call_args_list[1][0][0] == [ | ||
| "config", | ||
| "--local", | ||
| "user.signingkey", | ||
| "ABCD1234", | ||
| ] |
There was a problem hiding this comment.
The tests use short/invalid fingerprints (e.g., "ABCD1234") in the keyring and GPG outputs, but the signing module generates/validates 40-character hex fingerprints. Using non-40-hex fixtures can mask bugs around fingerprint normalization/validation; update the test data to use realistic 40-char hex fingerprints consistently (including the expected git config values).
| fingerprint = get_key_fingerprint() | ||
| if not fingerprint: | ||
| raise GPGError("No agent GPG key found. Run generate_agent_key() first.") | ||
|
|
||
| # Verify the exact secret key fingerprint is still present in the GPG keyring | ||
| stdout, _, returncode = _run_gpg_command( | ||
| ["--list-secret-keys", "--with-colons", fingerprint] | ||
| ) | ||
| fingerprint_found = any( | ||
| line.startswith("fpr:") and line.split(":")[9] == fingerprint | ||
| for line in stdout.splitlines() | ||
| ) |
There was a problem hiding this comment.
configure_git_signing() compares the stored fingerprint against the --with-colons output case-sensitively (line.split(':')[9] == fingerprint). If the keyring value is lower-case (or otherwise not normalized), this can incorrectly report the secret key as missing. Normalize the fingerprint (e.g., .upper()) before both the GPG call and the comparison (and ideally reuse the same normalization in get_key_fingerprint()).
| """ | ||
| try: | ||
| fingerprint = keyring.get_password(KEYRING_SERVICE, KEYRING_USERNAME) | ||
| return fingerprint |
There was a problem hiding this comment.
get_key_fingerprint() claims to return a 40-character hex fingerprint, but currently returns whatever the keyring contains without validation/normalization. Consider validating (len==40 and hex) and normalizing (uppercase) here; if invalid, return None (or raise a specific error) so downstream commands don’t fail with confusing GPG errors.
| return fingerprint | |
| if fingerprint is None: | |
| return None | |
| fingerprint = fingerprint.strip() | |
| if len(fingerprint) != 40: | |
| return None | |
| try: | |
| int(fingerprint, 16) | |
| except ValueError: | |
| return None | |
| return fingerprint.upper() |
| - [Telemetry Guide](./telemetry-guide.md) - Telemetry integration with GPG fingerprints | ||
| - [Security Workflow](./security-workflow.md) - Security scanning and compliance | ||
| - [Git Hooks](../reference/git-hooks.md) - Custom hooks for commit signing |
There was a problem hiding this comment.
The “Related Documentation” links appear to be broken: ./telemetry-guide.md, ./security-workflow.md, and ../reference/git-hooks.md do not exist in the docs tree. Please update these to point at existing pages (or add the missing docs) to avoid dead links in published documentation.
| - [Telemetry Guide](./telemetry-guide.md) - Telemetry integration with GPG fingerprints | |
| - [Security Workflow](./security-workflow.md) - Security scanning and compliance | |
| - [Git Hooks](../reference/git-hooks.md) - Custom hooks for commit signing | |
| - [Security Best Practices](#security-best-practices) - Guidance on telemetry monitoring and protecting agent signing keys | |
| - [Key Rotation](#key-rotation) - Operational guidance for rotating keys after compromise or on schedule | |
| - [Git Signing Documentation](https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work) - Git commit-signing behavior and related configuration |
…ixes Add `flowspec doctor` health-check command and `flowspec gpg` agent signing module, fully addressing all Copilot findings from PRs #1236, #1237, #1238, and #1239 plus a thorough independent security review. Key design decisions in signing module: - Fingerprint normalization centralized in get_key_fingerprint() (strip, uppercase, 40-char hex validation, logging on failure) - Key generation uses --status-fd for reliable fingerprint extraction - Secret key verification uses exact fingerprint match (case-insensitive) - All test mocks use realistic 40-char hex fingerprints via TEST_FP constant Doctor module improvements: - httpx imported at module level (coding standard compliance) - Strict flow.<phase>.agent.md validation (rejects flow.agent.md) - UTF-8 encoding on all file I/O - Version check distinguishes "confirmed current" from "could not check" Documentation quality: - All GPG commands use fingerprint (not email) to avoid ambiguity - Broken related-doc links replaced with anchor links - Security note about ~/.gnupg file permissions added - Manual deletion warns about keyring cleanup Closes #1221 #1222 #1223 #1224 #1225 #1226 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: jason poley <jason.poley@gmail.com>
…ixes Add `flowspec doctor` health-check command and `flowspec gpg` agent signing module, fully addressing all Copilot findings from PRs #1236, #1237, #1238, and #1239 plus a thorough independent security review. Key design decisions in signing module: - Fingerprint normalization centralized in get_key_fingerprint() (strip, uppercase, 40-char hex validation, logging on failure) - Key generation uses --status-fd for reliable fingerprint extraction - Secret key verification uses exact fingerprint match (case-insensitive) - All test mocks use realistic 40-char hex fingerprints via TEST_FP constant Doctor module improvements: - httpx imported at module level (coding standard compliance) - Strict flow.<phase>.agent.md validation (rejects flow.agent.md) - UTF-8 encoding on all file I/O - Version check distinguishes "confirmed current" from "could not check" Documentation quality: - All GPG commands use fingerprint (not email) to avoid ambiguity - Broken related-doc links replaced with anchor links - Security note about ~/.gnupg file permissions added - Manual deletion warns about keyring cleanup Closes #1221 #1222 #1223 #1224 #1225 #1226 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: jason poley <jason.poley@gmail.com>
Summary
Adds
flowspec doctorhealth-check command andflowspec gpgagent signing module with full CLI wiring, tests, and documentation.All Copilot review findings from PRs #1236, #1237, and #1238 are resolved in this single DCO-signed commit.
What's included
flowspec doctorcommand:flow.*.agent.mdwith strict 4-part validation), constitution presenceflowspec gpgsubcommands (setup/status/rotate):--status-fdfor reliable fingerprint extractionCopilot findings resolved (all 3 rounds):
--status-fdinstead of--list-keys(avoids wrong key with duplicate emails)flow.<phase>.agent.mdpattern validation (rejectsflow.agent.md,flow..agent.md)httpx.getin doctor tests (no network dependency)stderrvariable fixed (Ruff F841)gpgsub-app anddoctorcommand registered in main appCloses #1221 #1222 #1223 #1224 #1225 #1226
Test plan
uv run ruff check .— all checks passeduv run ruff format --check .— all files formatteduv run pytest tests/ -x -q— 3526 passed, 42 skipped, 0 failures🤖 Generated with Claude Code