refactor: auto-generate flag tables and replace --format with global --json#80
refactor: auto-generate flag tables and replace --format with global --json#80danielewood merged 8 commits intomainfrom
Conversation
…--json Add gendocs tool that generates README flag tables from Cobra command definitions via `go generate`, with CI check and pre-commit hook to keep them in sync. Extract ValidationError to dedicated errors.go. Replace per-command `--format text|json` flag on inspect, verify, connect, scan, ocsp, and crl with a global `--json` persistent flag. Rename `convert --to` to `convert --format` for consistency with `bundle --format` (both specify container format). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces automated generation of CLI flag tables in README.md from Cobra command definitions, replacing the per-command --format text|json flags with a global --json boolean flag, and renaming convert --to to convert --format for consistency. It also extracts the ValidationError type to a dedicated errors.go file for better organization.
Changes:
- Added
gendocstool with build tag that auto-generates markdown flag tables from Cobra commands viago generate, with CI and pre-commit enforcement - Replaced per-command
--formatflags with a global--jsonpersistent flag oninspect,verify,connect,scan,ocsp, andcrlcommands - Renamed
convert --totoconvert --formatfor consistency withbundle --format
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
doc.go |
Adds go:generate directive to run gendocs tool |
cmd/certkit/main.go |
Adds build tag !gendocs to exclude from gendocs build |
cmd/certkit/errors.go |
Extracts ValidationError type from main.go |
cmd/certkit/gendocs.go |
New tool that generates markdown flag tables from Cobra command definitions |
cmd/certkit/root.go |
Adds global --json persistent flag and updates flag descriptions with backticks |
cmd/certkit/inspect.go |
Removes local --format flag and init function; uses global jsonOutput variable |
cmd/certkit/verify.go |
Removes local --format flag; uses global jsonOutput; updates flag descriptions |
cmd/certkit/connect.go |
Removes local --format flag; uses global jsonOutput |
cmd/certkit/scan.go |
Removes local --format flag; uses global jsonOutput; reorders flag initialization |
cmd/certkit/ocsp.go |
Removes local --format flag; uses global jsonOutput; updates example to use --json |
cmd/certkit/crl.go |
Removes local --format flag; uses global jsonOutput; updates example to use --json |
cmd/certkit/convert.go |
Renames --to flag to --format; updates all references and examples |
cmd/certkit/bundle.go |
Updates flag descriptions with backticks; adds readme_default annotation |
cmd/certkit/sign.go |
Reorders flags; adds readme_default annotations; updates flag descriptions |
cmd/certkit/keygen.go |
Updates flag descriptions with backticks; adds readme_default annotation |
cmd/certkit/csr.go |
Updates flag descriptions; adds readme_default annotation |
README.md |
Auto-generated flag tables with markers; reflects all CLI flag changes |
EXAMPLES.md |
Updates all examples to use --json instead of --format json and --format instead of --to |
CLAUDE.md |
Updates CI check count to 11; adds G-8 requirement; updates CLI-3 and CLI-7 to reference --json |
CHANGELOG.md |
Documents breaking changes and new auto-generation feature |
.pre-commit-config.yaml |
Adds gendocs hook that runs on relevant file changes |
.github/workflows/ci.yml |
Adds docs CI job that verifies flag tables are up to date |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Keep per-command --format on display commands (inspect, verify, connect, scan, ocsp, crl) while adding a global --json persistent flag that works on ALL commands. --json overrides --format when both are set. Add JSON output to keygen, csr, sign (self-signed and csr), bundle, and convert commands. Revert convert --format back to --to. Fix CHANGELOG refs and add missing PR references. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
- Fix connect JSON sha256_fingerprint to use colon-hex format matching inspect and sha1_fingerprint (CLI-4 consistency) - Fix bundle --json to base64 encode binary p12/jks output instead of casting raw bytes to string (produces valid JSON) - Fix sign --json + -o to write file AND output JSON (matching bundle/convert behavior; previously --json silently skipped file write) - Simplify redundant conditionals in keygen/csr JSON output (omitempty handles empty strings) - Move --json changelog entries from Changed to Added section (CL-2) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add spliceMarkerInput struct for 3-parameter spliceMarker (CS-5) - Replace fmt.Fprintf(os.Stderr) with slog calls for diagnostics (OBS-1) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
All 4 findings addressed in commits 5ac49bf and b5aed21:
Additionally fixed in b5aed21:
|
- Standardize --json override to use local variable copy instead of mutating package-level format variables (all 6 display commands) - Decouple keygen/csr stderr file messages from --json (always print when -o is set, regardless of --json) - Add file path fields to keygenJSON and csrJSON for script discovery - Fix bundle --json -o with PEM format to emit file metadata instead of full PEM data (matches convert pattern) - Add doc comment on connectCertJSON struct - Add breaking change note for connect sha256_fingerprint format change Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
pflag's UnquoteUsage consumes backtick-quoted text as type placeholders, corrupting --help output for --format, --trust-store, --log-level, --algorithm, and --curve flags. Remove all backticks from usage strings. Rename csr --cert to --from-cert for clarity — avoids confusion with certificate file arguments used by other commands. Also fix convert --json without -o missing the format field in output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- github.com/spf13/pflag v1.0.9 → v1.0.10 - golang.org/x/exp → 2025-02-18 - modernc.org/libc v1.67.6 → v1.68.0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
cmd/certkit/convert.go:55
- PR description says
convert --towas renamed toconvert --format, but this change doesn't appear to be implemented: the flag is still named--to, help text/examples still refer to--to, and completion is registered forto. Either implement the rename (and update README/examples/completions) or adjust the PR description to avoid claiming this breaking change.
var convertCmd = &cobra.Command{
Use: "convert <file>",
Short: "Convert certificates and keys between formats",
Long: `Convert certificates and keys between PEM, DER, PKCS#12, JKS, and PKCS#7.
Input format is auto-detected. Use --to to specify the output format.
Binary formats (p12, jks) require -o to write to a file.
When --key is provided, convert matches keys to leaf certificates and builds
chain bundles. If multiple keys match different certs, JKS output creates a
multi-alias keystore. PKCS#12 supports only a single key entry.`,
Example: ` certkit convert cert.der --to pem
certkit convert cert.pem --to der -o cert.der
certkit convert cert.pem --key key.pem --to p12 -o bundle.p12
certkit convert bundle.p12 --to pem
certkit convert cert.pem --to p7b -o certs.p7b
certkit convert bundle.p12 --to jks -o keystore.jks`,
Args: cobra.ExactArgs(1),
RunE: runConvert,
}
func init() {
convertCmd.Flags().StringVar(&convertTo, "to", "", "Output format: pem, der, p12, jks, p7b")
convertCmd.Flags().StringVarP(&convertOutFile, "out-file", "o", "", "Output file (required for binary formats)")
convertCmd.Flags().StringVar(&convertKeyPath, "key", "", "Private key file (PEM). Keys are matched to certificates automatically.")
_ = convertCmd.MarkFlagRequired("to")
convertCmd.Flags().Lookup("out-file").Annotations = map[string][]string{"readme_default": {"_(stdout for PEM)_"}}
registerCompletion(convertCmd, completionInput{"to", fixedCompletion("pem", "der", "p12", "jks", "p7b")})
registerCompletion(convertCmd, completionInput{"out-file", fileCompletion})
registerCompletion(convertCmd, completionInput{"key", fileCompletion})
}
spliceMarker error messages hardcoded "README.md" instead of using the path argument. Include the path field in spliceMarkerInput so errors reference the correct file when gendocs is run with a custom path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
OBS-1 (MUST): This diagnostic message uses All other diagnostic paths in this function correctly use
This single line is inconsistent and breaks the exclusive- certkit/cmd/certkit/gendocs.go Lines 78 to 82 in fc4c4c8 |
|
Bug (CLI-4): The The other two branches both set
The CHANGELOG entry for this PR explicitly documents the same fix for Lines 138 to 143 in fc4c4c8 |
|
OBS-1 (MUST): Use File: This certkit/cmd/certkit/gendocs.go Lines 73 to 79 in fc4c4c8 From CLAUDE.md OBS-1: |
|
Bug: format field missing from JSON output when bundling to PEM stdout File: cmd/certkit/bundle.go Line: 141 (RIGHT side) The else branch (PEM output to stdout, no -o flag) never sets out.Format, so it is silently omitted from the JSON due to omitempty. The other two branches both set out.Format = bundleFormat correctly, and the analogous convert.go code (fixed in this same PR) also sets out.Format in its else branch. Lines 138 to 143 in fc4c4c8 The CHANGELOG even notes fixing the same pattern in convert - but the equivalent fix was not applied here. Suggested fix: Add out.Format = bundleFormat in the else branch, so the JSON output consistently includes the format field regardless of which output path is taken. |
|
Security: Private key emitted to stdout even when -o writes it to a file File: cmd/certkit/keygen.go Line: 73 (RIGHT side) When --json and -o are both specified, the full key_pem (private key) is always included in the JSON output to stdout, regardless of whether the key was already written to a file. Without --json, nothing is written to stdout when -o is used. The new --json flag unexpectedly creates a second output channel for the private key. Lines 69 to 86 in fc4c4c8 The same issue affects csr.go and sign.go (for sign self-signed). Compare with the bundle command in this same PR, which explicitly uses the pattern "File was written - emit metadata only" when -o is specified - omitting the raw data and instead returning only File, Format, and Size. The keygen/csr/sign commands should follow the same pattern: when -o is specified, include only file path metadata in the JSON (not the PEM content). From CLAUDE.md SEC-2: "Never log secrets (private keys, passwords)" - stdout in CI/CD environments is commonly captured in logs, making this an unintended exposure path. |
|
Bug: Binary guard blocks --json before the JSON path is reached; binary data would also be corrupted File: cmd/certkit/convert.go Line: 129 (RIGHT side) This guard runs unconditionally before the if jsonOutput block (line ~155), so "certkit convert cert.pem --to p12 --json" fails with an error even though --json could safely return base64-encoded binary data - exactly the behavior "bundle --json" provides. certkit/cmd/certkit/convert.go Lines 127 to 133 in fc4c4c8 Compare with bundle.go in this PR, where isBinary is checked inside the if jsonOutput block and correctly base64-encodes the data. Additionally, even if the guard were moved, line 164 uses out.Data = string(output) which would corrupt binary bytes (P12/JKS/DER/P7B) by treating them as UTF-8. The fix should:
|
|
Bug: When Compare with With this missing assignment, |
Code reviewTwo issues found. 1. Bug:
|
Summary
gendocstool that auto-generates README flag tables from Cobra command definitions viago generate, with a CI check and pre-commit hook to keep them in syncValidationErrorto dedicatederrors.gofile--jsonpersistent flag — all commands now support JSON output; overrides per-command--formatwhen both are setkeygen,csr,sign,bundle, andconvertcommands (previously text-only)--format text|jsonflags retained on display commands (inspect,verify,connect,scan,ocsp,crl)connectJSONsha256_fingerprintformat changed from lowercase hex to colon-separated uppercase hex for consistency withinspectandsha1_fingerprint(CLI-4)csr --certto--from-certfor clarity--format,--trust-store,--log-level,--algorithm, and--curveflags now display correctly in--helpTest plan
go build ./...passesgo test -race -count=1 ./...passesgo vet ./...cleangolangci-lint run— 0 issuesgo generate ./...— README up to datecertkit inspect cert.pem --jsonworkscertkit bundle cert.pem --format p12still workscertkit convert cert.pem --to pemworks--helpoutput has no backtick artifacts on any command🤖 Generated with Claude Code