Skip to content

Add subkey cap with template-method verification chain - #147

Merged
bmarwell merged 4 commits into
mainfrom
fix/subkey-cap-verification
May 26, 2026
Merged

Add subkey cap with template-method verification chain#147
bmarwell merged 4 commits into
mainfrom
fix/subkey-cap-verification

Conversation

@bmarwell

@bmarwell bmarwell commented May 25, 2026

Copy link
Copy Markdown
Owner

Summary

Verification design

  • AbstractKeyServerCommandHandler.execute(...) now always runs a command verification registry before doExecute(...)
  • CommandVerificationRegistry<T, V> is the typed registry interface for command-level pre-execution verification
  • VerificationStep<I, O> is the small-step interface for reusable validators/parsers
  • the add command uses KeyTextSizeValidator -> KeyTextParser -> KeyRingVerifier
  • VerifyUidCommandHandler uses a no-op registry so the execution flow stays consistent across handlers

Closes #136

@codecov

codecov Bot commented May 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.69925% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.94%. Comparing base (77727e8) to head (b06d2c7).

Files with missing lines Patch % Lines
...cmdhandler/verification/KeySubmissionVerifier.java 69.44% 5 Missing and 6 partials ⚠️
.../core/cmdhandler/verification/KeyRingVerifier.java 84.78% 4 Missing and 3 partials ⚠️
...ndler/AddKeyToVerificationQueueCommandHandler.java 72.22% 4 Missing and 1 partial ⚠️
...on/core/cmdhandler/verification/KeyTextParser.java 60.00% 2 Missing ⚠️
.../cmdhandler/verification/KeyTextSizeValidator.java 83.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #147      +/-   ##
============================================
+ Coverage     52.15%   59.94%   +7.78%     
- Complexity       69       88      +19     
============================================
  Files            21       27       +6     
  Lines           278      342      +64     
  Branches         32       37       +5     
============================================
+ Hits            145      205      +60     
- Misses          115      119       +4     
  Partials         18       18              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bmarwell bmarwell changed the title Add subkey-cap verification chain Add template-method verification chain May 25, 2026
@bmarwell
bmarwell force-pushed the fix/subkey-cap-verification branch from 3eb72a7 to 09385ae Compare May 25, 2026 21:39
@bmarwell
bmarwell requested a review from Copilot May 25, 2026 21:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 introduces a structured, template-method-based verification flow for command handlers, moving add-path validation into a typed verification registry/chain and adding a dedicated exception for subkey-cap enforcement.

Changes:

  • Added a template-method verification hook to AbstractKeyServerCommandHandler and introduced a typed CommandVerificationRegistry plus reusable VerificationStep building blocks.
  • Refactored /pks/add handling to run a verification chain (KeyTextSizeValidatorKeyTextParserKeyRingVerifier) and added a TooManySubkeysException for subkey-cap rejection.
  • Updated unit tests to use the new execute(...) flow and documented the new validation architecture.

Reviewed changes

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

Show a summary per file
File Description
docs/implementation-plan.adoc Documents the new verification hook/registry rollout plan.
docs/architecture.adoc Adds a description of the new validation architecture to the verification-flow docs.
application/application-core/src/test/java/io/github/bmarwell/keyserver/application/core/cmdhandler/VerifyUidCommandHandlerTest.java Updates tests to call execute(...) instead of doExecute(...).
application/application-core/src/test/java/io/github/bmarwell/keyserver/application/core/cmdhandler/verification/KeyRingVerifierTest.java Adds focused tests for UID/subkey cap enforcement in KeyRingVerifier.
application/application-core/src/test/java/io/github/bmarwell/keyserver/application/core/cmdhandler/AddKeyToVerificationQueueCommandHandlerTest.java Updates add-handler tests to use the new execution/verification flow.
application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/cmdhandler/VerifyUidCommandHandler.java Adapts handler to template-method verification (no-op registry) and updated doExecute signature.
application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/cmdhandler/verification/VerificationStep.java Introduces a small-step verification interface.
application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/cmdhandler/verification/NoOpCommandVerificationRegistry.java Adds a default registry implementation for handlers without verification steps yet.
application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/cmdhandler/verification/NoCommandVerification.java Adds a marker verification result for no-op verification.
application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/cmdhandler/verification/KeyTextSizeValidator.java Extracts keytext-size validation into a reusable verification step.
application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/cmdhandler/verification/KeyTextParser.java Extracts armored-key parsing into a verification step.
application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/cmdhandler/verification/KeySubmissionVerifier.java Implements the add-path verification registry/chain.
application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/cmdhandler/verification/KeyRingVerifier.java Adds UID/subkey verification logic and produces prepared verification output.
application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/cmdhandler/verification/CommandVerificationRegistry.java Introduces a typed command-level verification registry API.
application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/cmdhandler/AddKeyToVerificationQueueCommandHandler.java Refactors add-handler to use the verification registry and consume verified output.
application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/cmdhandler/AbstractKeyServerCommandHandler.java Adds the template-method execute(...) flow: verify first, then doExecute(...).
application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/ex/TooManySubkeysException.java Adds a dedicated exception type for subkey-cap violations.
application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/ex/KeyValidationException.java Extends the sealed exception hierarchy to permit TooManySubkeysException.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/implementation-plan.adoc Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bmarwell bmarwell changed the title Add template-method verification chain Add subkey cap with template-method verification chain May 25, 2026
@bmarwell
bmarwell requested a review from Copilot May 25, 2026 21:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 18 out of 18 changed files in this pull request and generated 4 comments.

bmarwell and others added 2 commits May 26, 2026 00:01
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 19 out of 19 changed files in this pull request and generated 1 comment.

@bmarwell
bmarwell merged commit 5de2003 into main May 26, 2026
7 checks passed
@bmarwell
bmarwell deleted the fix/subkey-cap-verification branch May 26, 2026 08:05
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.

fix: add subkey count cap in AddKeyToVerificationQueueCommandHandler

2 participants