feat: add shell tab completion for all enum and directory flags - #56
Merged
Conversation
Register Cobra flag completion functions for all enum flags (--format, --algorithm, --curve, --log-level, --trust-store) and directory flags (--bundle-path, --out-path) across all commands. Add completions.go with fixedCompletion and directoryCompletion helpers. Document shell completion setup in README. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
danielewood
force-pushed
the
feat/shell-completion
branch
from
February 21, 2026 06:00
c3cae50 to
174853a
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds shell tab completion support for CLI flags across all certkit commands. The feature enables tab completion for enum-valued flags (format, algorithm, curve, log-level, trust-store) and directory path flags (bundle-path, out-path) in bash, zsh, fish, and PowerShell.
Changes:
- Added completion helper functions (
fixedCompletionanddirectoryCompletion) in newcmd/certkit/completions.gofile - Registered completion handlers for enum and directory flags across all 7 commands
- Documented shell completion setup instructions in README.md
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cmd/certkit/completions.go | New file with helper functions for fixed-value and directory completions |
| cmd/certkit/root.go | Added completion for --log-level flag |
| cmd/certkit/scan.go | Added completions for --format and --bundle-path flags |
| cmd/certkit/bundle.go | Added completions for --format and --trust-store flags |
| cmd/certkit/verify.go | Added completions for --format and --trust-store flags |
| cmd/certkit/inspect.go | Added completion for --format flag |
| cmd/certkit/keygen.go | Added completions for --algorithm, --curve, and --out-path flags |
| cmd/certkit/csr.go | Added completions for --algorithm, --curve, and --out-path flags |
| README.md | Added Shell Completion section with installation instructions for all supported shells |
| CHANGELOG.md | Added entry documenting the new shell completion feature |
Bump sensiblebit/.github rev to bb8938c which adds the golangci-lint hook, and enable it in this repo's pre-commit config. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Also fix hook ordering and naming to match actual .pre-commit-config.yaml. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move golangci-lint hook before go-build and go-test for faster feedback. Bump sensiblebit/.github rev to 15d19ac which routes golangci-lint through checks.py for a require_tool guard. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move go-vet and golangci-lint before wasm — fast static analysis on the primary target should run before the secondary WASM build check. Final order: formatters (goimports, go-fix) → static analysis (go-vet, golangci-lint) → secondary target (wasm) → build → test. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
…ook ID Rename the opaque single-letter field name to be descriptive per naming conventions. Fix CLAUDE.md hook ID from "wrangler build" to "wrangler-build" to match the actual .pre-commit-config.yaml hook ID. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
1 similar comment
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
danielewood
added a commit
that referenced
this pull request
Feb 21, 2026
Add ([#56]) refs to 7 unreleased entries that were missing them. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
danielewood
added a commit
that referenced
this pull request
Feb 21, 2026
Add ([#56]) refs to 7 unreleased entries that were missing them. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
danielewood
added a commit
that referenced
this pull request
Feb 21, 2026
* feat: add trust and expiry annotations to scan summary, inspect, and WASM Scan summary now shows expired and untrusted counts per cert type (e.g., "Leaves: 6 (2 expired, 1 untrusted)"). Missing intermediates are resolved via AIA before trust checking. Inspect output now includes Expired and Trusted fields for each certificate. Key changes: - Add IsMozillaRoot() to detect cross-signed roots whose signing CA was removed from the trust store (e.g., GTS Root R1 × GlobalSign) - Add HasUnresolvedIssuers() to conditionally trigger AIA resolution - Add ScanSummaryInput with RootPool and AllowExpired parameters - Add AnnotateInspectTrust() for per-certificate trust annotation - Fix WASM getState() to use ExtKeyUsageAny and handle cross-signed roots and expired cert time-shifting consistently with CLI - Replace fmt.Fprintf(os.Stderr) diagnostics with slog (OBS-1) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: strengthen trust verification, fix WASM race and error handling Extract shared VerifyChainTrust function to eliminate triplicated chain verification logic across scan summary, inspect, WASM, and --dump-certs. Strengthen IsMozillaRoot with public key verification to prevent spoofed trust anchors. Use NotBefore+1s time-shift for expired cert verification (more robust when intermediates expired before the leaf). Key fixes: - WASM globalStore race condition (add sync.RWMutex) - WASM MozillaRootPool error silently ignored - --dump-certs missing ExtKeyUsageAny and IsMozillaRoot bypass - Root certs always trust-checked regardless of --allow-expired - Bare return err in inspect command (ERR-1) Tests: IsMozillaRoot (genuine root, spoofed subject, unrelated cert), VerifyChainTrust (trusted chain, expired time-shift, untrusted, Mozilla root bypass), HasUnresolvedIssuers (all skip conditions), positive trust case in AnnotateInspectTrust, strengthened ScanSummaryTrust. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: harden AIA fetchers against SSRF and context cancellation Add ValidateAIAURL to reject non-HTTP schemes and literal private/loopback IP addresses. Apply to library FetchAIACertificates, CLI httpAIAFetcher, and redirect handlers (max 3 redirects). Fix WASM jsFetchURL to honor context cancellation via select on ctx.Done(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: return copy from AllKeys, extract certAnnotation, add --out-file completion AllKeys() now returns a copy of the internal map (like AllCerts()) so callers cannot corrupt store state. Extract certAnnotation to internal.CertAnnotation for testability. Add fileCompletion for --out-file flag on the bundle command. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: strengthen trust/expiry assertions across all test suites Expand test coverage for trust and expiry annotations across packages: - ScanSummaryTrust: assert all 6 fields, add expired root case - AnnotateInspectTrust: add chain case exercising intermediate pool - HasUnresolvedIssuers: add Mozilla-root-issued leaf case - VerifyChainTrust: add not-yet-valid leaf case, document expired intermediate limitation in doc comment - EmptyStore: add trust/expiry zero-checks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: prevent expired+untrusted double-counting in scan summary Expired certificates were counted as both expired and untrusted, misleading users about the number of actual trust issues. Now expired certs are only counted as expired — trust checking is skipped for them regardless of the AllowExpired setting. Remove the now-unused AllowExpired field from ScanSummaryInput. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: add missing PR refs to CHANGELOG entries Add ([#56]) refs to 7 unreleased entries that were missing them. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: close SSRF bypass, WASM panic, callback leak, and test gaps - Block unspecified addresses (0.0.0.0, ::) in ValidateAIAURL SSRF check - Avoid releasing js.Func callbacks on context cancellation in jsFetchURL to prevent panic when JS promise settles after Go cancels - Release setTimeout callback in addFiles to prevent per-call leak - Add test cases for IPv6 ULA, unspecified addresses, untrusted intermediate counting in ScanSummary - Fix silent error suppression in TestAnnotateInspectTrust by reusing newIntermediateCA helper - Use atomic.Int32 for fetchCount in AIA tests (consistency with bundle_test.go) - Use parsed certs instead of templates as x509.CreateCertificate parents in AIA max depth test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: harden SSRF, WASM deadlock, memory leaks, and nil-roots fallback Adversarial review found seven issues in the trust-annotations PR: - ValidateAIAURL re-parsed CIDR ranges on every call and missed 100.64.0.0/10 (CGN/shared address space). Move to package init. - ResolveAIA passed raw AIA URLs to the fetcher without SSRF validation — the WASM jsFetchURL path had no URL checks at all. - VerifyChainTrust silently fell back to system roots when roots was nil (x509.VerifyOptions behavior). Now returns false. - WASM getState/resetStore blocked the JS event loop when AIA held the store lock, causing a deadlock. Use TryRLock/TryLock instead. - WASM jsFetchURL accepted unbounded response data. Add 1MB limit. - WASM addFiles/exportBundlesJS/jsError leaked js.FuncOf promise executors. Release immediately after Promise.New (called sync). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: harden error handling, WASM safety, and test coverage gaps Adversarial review found ERR-1/ERR-5 violations, a WASM catch callback null-safety issue, a missing defer on RUnlock, and several untested code paths. This fixes all production code issues and adds targeted tests. Production fixes: - Wrap bare return err in scan.go MozillaRootPool and httpAIAFetcher (ERR-1) - Add slog.Debug for silent continues in mozillaRootPublicKeys and ResolveAIA (ERR-5) - Log json.Marshal errors in WASM instead of silently discarding (ERR-5) - Guard WASM catchCb against null/non-Error JS rejections - Use defer for RUnlock in exportBundlesJS to prevent deadlock on panic - Add context prefix to WASM AIA fetch error messages Test additions: - ResolveAIA SSRF URL rejection (loopback, private, file scheme) - VerifyChainTrust with nil intermediates pool and expired intermediate - ScanSummary nil-pool with expired certs - Strengthen ValidateAIAURL empty-scheme assertion Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: document accepted risks for DNS rebinding and shared map Explain why ValidateAIAURL intentionally allows hostnames that resolve to private IPs (short-lived CLI, internal CA support, Control callback has same TOCTOU) and why MozillaRootSubjects returns the backing map directly (all callers read-only, defensive copy wastes allocations). Both were flagged in a prior adversarial review and accepted as known limitations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address review findings — CS-5, ERR-1, ERR-5 compliance - Refactor VerifyChainTrust to accept VerifyChainTrustInput struct (CS-5: more than 2 arguments requires input struct) - Wrap bare io.ReadAll return in httpAIAFetcher with context (ERR-1) - Reject WASM addFiles promise on json.Marshal failure instead of resolving with empty string (ERR-5) - Add slog.Debug for silent continue in MozillaRootSubjects (ERR-5) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Feb 24, 2026
This was referenced Mar 9, 2026
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
Shell completion
RegisterFlagCompletionFunccalls for all enum flags (--format,--algorithm,--curve,--log-level,--trust-store), directory flags (--bundle-path,--out-path), and file flags (--out-file) across all commandscmd/certkit/completions.gowithfixedCompletion,directoryCompletion, andfileCompletionhelpersTrust and expiry annotations
Leaves: 6 (2 expired, 1 untrusted))inspectcommand outputVerifyChainTrustshared function for consistent chain verification across CLI, WASM, inspect, and--dump-certsIsMozillaRootto verify public key in addition to SubjectSecurity hardening
ValidateAIAURLto block SSRF via non-HTTP schemes and literal private/loopback IPs in AIA URLsjsFetchURLignoring context cancellationglobalStorerace condition withsync.RWMutexBug fixes
AllKeys()returning internal map (callers could corrupt store state)--dump-certsusing inconsistent chain verificationgetStatesilently ignoringMozillaRootPool()errorcertkit inspectbarereturn errwithout context wrappingTests
ScanSummaryTrustto assert all 6 trust/expiry fields with expired root caseAnnotateInspectTrustexercising intermediate pool buildingHasUnresolvedIssuersVerifyChainTrustCertAnnotationto internal package for testabilityTest plan
go build ./...,go vet ./...,go test -race ./...passgolangci-lint runpassesGOOS=js GOARCH=wasm go vet ./cmd/wasm/andgo buildpasscertkit __complete scan --format ""returnstext,jsoncertkit __complete bundle --out-file ""triggers file completion🤖 Generated with Claude Code