# Implementation Plan: SDK Review Hardening

**Branch**: `002-sdk-review-hardening` | **Date**: 2026-01-17 | **Spec**: [spec.md](./spec.md)
**Input**: Multi-perspective adversarial code review findings

## Summary

Address 17 violations identified by 8 adversarial reviewer personas (Open Source Advocate, Rust Purist, Quality Engineer, Linus Torvalds, Junior Dev, Security Architect, Network Architect, Amazon WebDev) before public SDK release. Priority focuses on 5 P0 security blockers, then P1 code idioms and test coverage, then P2 networking and integration improvements.

## Technical Context

**Language/Version**: Rust 1.85+ (2024 edition)
**Primary Dependencies**: reqwest, p256, jwt-simple, serde, mini-moka/micro-moka, tokio, secrecy, zeroize, thiserror
**Storage**: TOML config files, environment variables (no persistent storage)
**Testing**: cargo test, cargo llvm-cov (80% threshold), wasm-pack test, proptest
**Target Platform**: Linux (glibc/musl), macOS (x86_64/aarch64), Windows, WASM
**Project Type**: Single library crate
**Performance Goals**: N/A (hardening focus)
**Constraints**: 80% test coverage, zero clippy warnings, no unsafe code
**Scale/Scope**: ~15,000 LOC, 54 source files

## Constitution Check

*GATE: All principles align with this feature.*

| Principle | Status | Notes |
|-----------|--------|-------|
| I. Customer-Facing Quality | SUPPORTS | Fixing issues before public release |
| II. Test Coverage (95%) | SUPPORTS | TR-001-TR-006 improve real coverage |
| III. Code Cleanliness | SUPPORTS | FR-006 enforces zero clippy warnings |
| IV. Cross-Platform | SUPPORTS | TR-004 adds WASM tests |
| V. Safe Updates | SUPPORTS | Refactoring existing code carefully |
| VI. Maintainability | SUPPORTS | FR-014 extracts magic numbers |
| VII. Security-First | SUPPORTS | SEC-001-SEC-008 are core focus |

**Result**: PASS - No violations, all principles supported.

## Project Structure

### Documentation (this feature)

```text
specs/002-sdk-review-hardening/
├── spec.md              # Feature specification
├── plan.md              # This file
├── research.md          # Phase 0: Research findings
├── data-model.md        # Phase 1: New/modified entities
├── quickstart.md        # Phase 1: Implementation guide
├── checklists/
│   └── requirements.md  # Validation checklist
└── tasks.md             # Phase 2: Generated by /speckit.tasks
```

### Source Code (repository root)

```text
src/
├── auth/
│   ├── mod.rs
│   ├── token_manager.rs    # SEC-004: Add token binding verification
│   └── trait_def.rs        # Consolidate WASM/native traits
├── client/
│   ├── mod.rs
│   ├── builder.rs
│   └── lastid_client.rs    # SEC-005: Clock skew; FR-010: Timeout as Err
├── config/
│   ├── mod.rs              # Newtype adoption, NetworkConfig
│   ├── network.rs          # NEW: FR-007, FR-008 proxy/timeout config
│   └── *.rs
├── constants.rs            # NEW: FR-014 named constants
├── crypto/
│   ├── dpop.rs             # SEC-002, SEC-003: RNG fix, zeroize fix
│   └── secrets.rs
├── error/
│   ├── mod.rs              # FR-005: #[non_exhaustive], new variants
│   └── *.rs
├── http/
│   ├── client.rs           # FR-007: Proxy support
│   ├── idp_client.rs       # FR-011: Correlation IDs
│   └── *.rs
├── policies/
│   └── *.rs                # SEC-001: HTTPS enforcement
├── trust_registry/
│   └── types.rs            # FR-004: Did newtype
├── types/
│   ├── mod.rs
│   ├── did.rs              # NEW: FR-004
│   ├── client_id.rs        # NEW: FR-004
│   ├── endpoint_url.rs     # NEW: SEC-001, FR-004
│   └── request_id.rs       # Existing newtype (good example)
└── verification/
    └── sdjwt.rs            # SEC-005: nbf validation

tests/
├── poll_and_verify.rs      # TR-001: Enable signature test
├── wasm/
│   └── mod.rs              # NEW: TR-004 WASM tests
└── *.rs

.github/workflows/
└── ci.yml                  # TR-003: Add example compilation

# Root files
CODE_OF_CONDUCT.md          # NEW: FR-001
SECURITY.md                 # NEW: FR-002
README.md                   # FR-003: Badges; DR-001: Fix defaults
lastid.toml.example         # NEW: DR-004
```

**Structure Decision**: Single library crate structure maintained. New files added for newtypes, constants, network config, and WASM tests.

## Complexity Tracking

> No constitution violations to justify.

## Implementation Phases

### Phase 1: P0 Security Blockers (SEC-001 to SEC-005)

| Task | Files | Effort |
|------|-------|--------|
| Fix zeroization | `src/crypto/dpop.rs` | Small |
| Fix RNG (remove Default) | `src/crypto/dpop.rs` | Small |
| Add EndpointUrl newtype | `src/types/endpoint_url.rs` (new) | Medium |
| Update URL validation | `src/policies/*.rs`, `src/config/mod.rs` | Medium |
| Add clock skew tolerance | `src/client/lastid_client.rs`, `src/config/mod.rs` | Medium |
| Add nbf validation | `src/verification/sdjwt.rs` | Small |
| Add token binding verification | `src/auth/token_manager.rs` | Medium |

### Phase 2: P1 Code Quality (FR-004 to FR-006, TR-001 to TR-004)

| Task | Files | Effort |
|------|-------|--------|
| Add Did newtype | `src/types/did.rs` (new) | Medium |
| Add ClientId newtype | `src/types/client_id.rs` (new) | Small |
| Update all String usages | ~10 files | Large |
| Add #[non_exhaustive] | ~15 enum definitions | Small |
| Extract magic numbers | `src/constants.rs` (new), ~8 files | Medium |
| Enable signature test | `tests/poll_and_verify.rs` | Medium |
| Add WASM tests | `tests/wasm/mod.rs` (new) | Medium |
| Fix coverage docs | `README.md` | Trivial |
| Add example CI step | `.github/workflows/ci.yml` | Small |

### Phase 3: P2 Networking & Integration (FR-007 to FR-012)

| Task | Files | Effort |
|------|-------|--------|
| Add NetworkConfig | `src/config/network.rs` (new) | Medium |
| Add proxy support | `src/http/client.rs` | Medium |
| Add granular timeouts | `src/http/client.rs`, `src/config/mod.rs` | Medium |
| Fix timeout return type | `src/client/lastid_client.rs` | Small |
| Add correlation IDs | `src/http/idp_client.rs` | Small |
| Add WebSocket fallback logging | `src/client/lastid_client.rs` | Trivial |

### Phase 4: Community Standards (FR-001 to FR-003, DR-001 to DR-006)

| Task | Files | Effort |
|------|-------|--------|
| Add CODE_OF_CONDUCT.md | Root (new) | Trivial |
| Add SECURITY.md | Root (new) | Small |
| Add README badges | `README.md` | Small |
| Fix README defaults | `README.md` | Trivial |
| Add lastid.toml.example | Root (new) | Small |
| Document MSRV policy | `README.md` | Trivial |

## Dependencies

```
Phase 1 (Security)
    │
    ├── EndpointUrl newtype ──┐
    │                         │
    └── Clock skew config ────┼──► Phase 2 (newtypes depend on validation)
                              │
Phase 2 (Code Quality)        │
    │                         │
    ├── All newtypes ─────────┘
    │
    └── Constants ─────────────► Phase 3 (networking uses constants)

Phase 3 (Networking)
    │
    └── NetworkConfig ─────────► Phase 4 (docs reference config)

Phase 4 (Community)
    │
    └── Independent (can parallelize with Phase 3)
```

## Risk Mitigation

| Risk | Mitigation |
|------|------------|
| Newtype adoption breaks existing code | Implement `From<String>` for migration, deprecate old APIs |
| Proxy support complexity | Use reqwest's built-in proxy support, test with mitmproxy |
| WASM test flakiness | Run in headless Chrome, use deterministic fixtures |
| Clock skew edge cases | Extensive property-based testing with proptest |

## Verification

After all phases:

```bash
# Security
cargo clippy --all-features -- -D warnings
cargo audit

# Tests
cargo test --all-features
cargo llvm-cov --all-features --fail-under 80
wasm-pack test --headless --chrome

# Docs
cargo doc --all-features --no-deps

# Examples
cargo build --examples --all-features
```

## Artifacts Generated

- [x] `research.md` - All findings validated
- [x] `data-model.md` - New newtypes and modified structures defined
- [x] `quickstart.md` - Implementation guide with code examples
- [ ] `tasks.md` - To be generated by `/speckit.tasks`
