Skip to content

feat: add shell tab completion for all enum and directory flags - #56

Merged
danielewood merged 9 commits into
mainfrom
feat/shell-completion
Feb 21, 2026
Merged

feat: add shell tab completion for all enum and directory flags#56
danielewood merged 9 commits into
mainfrom
feat/shell-completion

Conversation

@danielewood

@danielewood danielewood commented Feb 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

Shell completion

  • Add RegisterFlagCompletionFunc calls for all enum flags (--format, --algorithm, --curve, --log-level, --trust-store), directory flags (--bundle-path, --out-path), and file flags (--out-file) across all commands
  • Add cmd/certkit/completions.go with fixedCompletion, directoryCompletion, and fileCompletion helpers
  • Document shell completion setup (bash/zsh/fish/PowerShell) in README

Trust and expiry annotations

  • Add expired and untrusted certificate counts to scan summary output (e.g., Leaves: 6 (2 expired, 1 untrusted))
  • Add AIA resolution to scan summary — fetch missing intermediates before trust checking
  • Add expired and trusted status to inspect command output
  • Add VerifyChainTrust shared function for consistent chain verification across CLI, WASM, inspect, and --dump-certs
  • Strengthen IsMozillaRoot to verify public key in addition to Subject

Security hardening

  • Add ValidateAIAURL to block SSRF via non-HTTP schemes and literal private/loopback IPs in AIA URLs
  • Limit AIA redirect count to 3 and validate each redirect target
  • Fix WASM jsFetchURL ignoring context cancellation
  • Fix WASM globalStore race condition with sync.RWMutex

Bug fixes

  • Fix expired certificates double-counted as both expired and untrusted in scan summary
  • Fix AllKeys() returning internal map (callers could corrupt store state)
  • Fix --dump-certs using inconsistent chain verification
  • Fix WASM getState silently ignoring MozillaRootPool() error
  • Fix certkit inspect bare return err without context wrapping

Tests

  • Expand ScanSummaryTrust to assert all 6 trust/expiry fields with expired root case
  • Add chain test case to AnnotateInspectTrust exercising intermediate pool building
  • Add Mozilla-root-issued leaf case to HasUnresolvedIssuers
  • Add not-yet-valid leaf case to VerifyChainTrust
  • Extract CertAnnotation to internal package for testability

Test plan

  • go build ./..., go vet ./..., go test -race ./... pass
  • golangci-lint run passes
  • GOOS=js GOARCH=wasm go vet ./cmd/wasm/ and go build pass
  • Shell completion: certkit __complete scan --format "" returns text, json
  • Shell completion: certkit __complete bundle --out-file "" triggers file completion
  • All pre-commit hooks pass on every commit

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings February 21, 2026 05:56
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
danielewood force-pushed the feat/shell-completion branch from c3cae50 to 174853a Compare February 21, 2026 06:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (fixedCompletion and directoryCompletion) in new cmd/certkit/completions.go file
  • 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

Comment thread CHANGELOG.md Outdated
danielewood and others added 2 commits February 21, 2026 01:04
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>
Copilot AI review requested due to automatic review settings February 21, 2026 06:09
@claude

This comment has been minimized.

@claude

This comment has been minimized.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

@claude

This comment has been minimized.

danielewood and others added 2 commits February 21, 2026 01:16
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>
Copilot AI review requested due to automatic review settings February 21, 2026 06:18
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>
@claude

This comment has been minimized.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comment thread cmd/certkit/completions.go
Comment thread cmd/certkit/completions.go
…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>
@claude

This comment has been minimized.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 21, 2026 06:25
@claude

claude Bot commented Feb 21, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

@claude

claude Bot commented Feb 21, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

1 similar comment
@claude

claude Bot commented Feb 21, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@danielewood
danielewood merged commit 9168b7f into main Feb 21, 2026
19 checks passed
@danielewood
danielewood deleted the feat/shell-completion branch February 21, 2026 13:59
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants