b928be4f2f
Generated design docs and implementation plans via Codex for: - Batch 1: Proto, Const, CipherSuites, NKey, JWT - Batch 2: Parser, Sublist, MemStore remainders - Batch 3: SendQ, Service, Client ProxyProto - Batch 4: Logging - Batch 5: JetStream Errors - Batch 8: Store Interfaces All plans include mandatory verification protocol and anti-stub guardrails. Updated batches.md with file paths and planned status.
126 lines
5.3 KiB
Markdown
126 lines
5.3 KiB
Markdown
# Batch 1 (Proto, Const, CipherSuites, NKey, JWT) Design
|
|
|
|
**Date:** 2026-02-27
|
|
**Scope:** Design only for implementing Batch 1 feature ports (10 features, 0 tracked tests).
|
|
|
|
## Context Snapshot
|
|
|
|
Batch metadata (from `porting.db`):
|
|
|
|
- Batch ID: `1`
|
|
- Name: `Proto, Const, CipherSuites, NKey, JWT`
|
|
- Features: `10`
|
|
- Tests: `0`
|
|
- Dependencies: none
|
|
- Go files: `server/ciphersuites.go`, `server/const.go`, `server/jwt.go`, `server/nkey.go`, `server/proto.go`
|
|
|
|
Feature IDs in this batch:
|
|
|
|
- `384` `init` -> `CipherSuites.Init`
|
|
- `583` `init` -> `ServerConstants.Init`
|
|
- `1975` `wipeSlice` -> `JwtProcessor.WipeSlice`
|
|
- `2441` `Server.nonceRequired` -> `NatsServer.NonceRequiredInternal`
|
|
- `2593` `protoScanField` -> `ProtoWire.ProtoScanField`
|
|
- `2594` `protoScanTag` -> `ProtoWire.ProtoScanTag`
|
|
- `2595` `protoScanFieldValue` -> `ProtoWire.ProtoScanFieldValue`
|
|
- `2596` `protoScanVarint` -> `ProtoWire.ProtoScanVarint`
|
|
- `2597` `protoScanBytes` -> `ProtoWire.ProtoScanBytes`
|
|
- `2598` `protoEncodeVarint` -> `ProtoWire.ProtoEncodeVarint`
|
|
|
|
## Current Code Findings
|
|
|
|
1. `CipherSuites` and `ServerConstants` use static constructors, but no explicit `Init()` method exists.
|
|
2. `JwtProcessor.WipeSlice` does not exist; `AuthHandler.WipeSlice` exists.
|
|
3. `NatsServer.NonceRequiredInternal` does not exist; `NonceRequired()` exists and is currently stubbed to `false`.
|
|
4. `ProtoWire` behavior is largely implemented, but method names are currently `Scan*` / `EncodeVarint`, not `ProtoScan*` / `ProtoEncodeVarint` mapped in Batch 1.
|
|
5. There are existing tests for `CipherSuites` and `JwtProcessor`, but no dedicated `ProtoWire` tests and no direct nonce-requirement tests.
|
|
|
|
## Goals
|
|
|
|
- Deliver full behavioral parity for the 10 Batch 1 features.
|
|
- Align implementation names/signatures with tracker mappings so Roslyn audit can classify features correctly.
|
|
- Add/extend tests where needed so features can be verified from evidence, not by assumption.
|
|
|
|
## Non-Goals
|
|
|
|
- No broader auth/JWT full claim-validation implementation beyond this batch's mapped functions.
|
|
- No batch execution/status updates in this design document.
|
|
- No unrelated refactors.
|
|
|
|
## Approaches
|
|
|
|
### Approach A: Tracker-name wrappers only (minimal)
|
|
|
|
Implement only mapped method names as wrappers around current code; avoid deeper behavior changes.
|
|
|
|
- Pros: Fastest; lowest risk of regressions.
|
|
- Cons: Could preserve existing behavioral drift (for example nonce logic), leaving hidden parity gaps.
|
|
|
|
### Approach B: Full rename + strict parity rewrite
|
|
|
|
Rename existing methods to exact Go-style mapped names and reshape internals to match Go structure exactly.
|
|
|
|
- Pros: Maximum parity and audit friendliness.
|
|
- Cons: Higher churn and avoidable breakage risk for existing callers/tests.
|
|
|
|
### Approach C (Recommended): Hybrid parity + compatibility
|
|
|
|
Add mapped methods required by Batch 1 while preserving current call surfaces via wrappers or forwards, then close behavior gaps and strengthen tests.
|
|
|
|
- Pros: Satisfies audit mapping and behavior with limited churn.
|
|
- Cons: Slightly more code than Approach A.
|
|
|
|
## Recommended Design
|
|
|
|
### 1. API + Audit Alignment Layer
|
|
|
|
- Add explicit `Init()` methods for `CipherSuites` and `ServerConstants` (idempotent warm-up semantics).
|
|
- Add `JwtProcessor.WipeSlice(Span<byte>)` and keep compatibility with existing usages.
|
|
- Add `NatsServer.NonceRequiredInternal()` (lock-held semantics) and call it from `NonceRequired()`.
|
|
- Add `ProtoWire.ProtoScan*` / `ProtoEncodeVarint` methods; keep existing `Scan*` / `EncodeVarint` as forwarding aliases if needed.
|
|
|
|
### 2. Behavior Parity Layer
|
|
|
|
- Ensure nonce requirement logic matches Go intent:
|
|
- `AlwaysEnableNonce`
|
|
- configured nkeys present
|
|
- trusted keys present
|
|
- proxy key pairs present
|
|
- Ensure proto varint and field scanners keep existing overflow/insufficient-data behavior at boundaries.
|
|
|
|
### 3. Verification Layer (Test-first)
|
|
|
|
- Extend existing tests:
|
|
- `Auth/CipherSuitesTests.cs`
|
|
- `Auth/JwtProcessorTests.cs`
|
|
- `ServerTests.cs` (or a dedicated nonce test file)
|
|
- Add new focused tests for `ProtoWire` in `dotnet/tests/ZB.MOM.NatsNet.Server.Tests/Internal/ProtoWireTests.cs`.
|
|
- Keep xUnit 3 + Shouldly + NSubstitute only.
|
|
|
|
### 4. PortTracker Status Discipline
|
|
|
|
- Move features through `stub -> complete -> verified` only after build + test evidence.
|
|
- Use max 15 IDs per batch status update.
|
|
- Store command output evidence in commit/task notes before updates.
|
|
|
|
## Risks and Mitigations
|
|
|
|
1. **Audit false negatives due naming mismatch**
|
|
Mitigation: implement mapped method names explicitly (`Init`, `WipeSlice`, `NonceRequiredInternal`, `Proto*`).
|
|
|
|
2. **Regressions from changing existing method names**
|
|
Mitigation: prefer additive wrappers and keep old names temporarily where already referenced.
|
|
|
|
3. **Undertested low-level protobuf boundaries**
|
|
Mitigation: dedicated boundary tests for 1..10-byte varint decode and overflow paths.
|
|
|
|
4. **Stub creep under schedule pressure**
|
|
Mitigation: mandatory stub scans and hard gates in the implementation plan.
|
|
|
|
## Success Criteria
|
|
|
|
- All 10 Batch 1 features are implemented with mapped method presence and intended behavior.
|
|
- Related tests pass and include explicit coverage for nonce logic and proto wire helpers.
|
|
- No placeholder implementations (`NotImplementedException`, TODO stubs, empty methods) in touched feature/test files.
|
|
- Batch 1 status can be advanced using evidence-based verification.
|