# Tasks: SDK Review Hardening

**Input**: Design documents from `/specs/002-sdk-review-hardening/`
**Prerequisites**: plan.md, spec.md, research.md, data-model.md, quickstart.md

**Tests**: Test tasks included for TR-001 through TR-006 requirements.

**Organization**: Tasks grouped by user story priority (P0 → P1 → P2 → P3).

## Format: `[ID] [P?] [Story] Description`

- **[P]**: Can run in parallel (different files, no dependencies)
- **[Story]**: Which user story this task belongs to (e.g., US1, US2, etc.)
- Include exact file paths in descriptions

---

## Phase 1: Setup (Shared Infrastructure)

**Purpose**: Create foundational files and error variants used across all stories

- [X] T001 Create constants module with all named constants in src/constants.rs
- [X] T002 [P] Add new error variants (InvalidDid, EmptyClientId, InsecureUrl, ClockSkewExceeded, TokenBindingMismatch) to src/error/mod.rs
- [X] T003 [P] Add `#[non_exhaustive]` to LastIDError enum in src/error/mod.rs
- [X] T004 [P] Add `#[non_exhaustive]` to HttpError enum in src/error/http.rs
- [X] T005 [P] Add `#[non_exhaustive]` to TrustRegistryError enum in src/error/trust_registry.rs
- [X] T006 [P] Add `#[non_exhaustive]` to WebSocketError enum in src/error/websocket.rs

---

## Phase 2: Foundational (Newtypes - Blocking Prerequisites)

**Purpose**: Create newtypes required by security validation and code quality stories

**⚠️ CRITICAL**: Security fixes (US1) depend on EndpointUrl newtype for HTTPS enforcement

- [X] T007 Create Did newtype with format validation in src/types/did.rs
- [X] T008 [P] Create ClientId newtype with non-empty validation in src/types/client_id.rs
- [X] T009 [P] Create EndpointUrl newtype with HTTPS enforcement (localhost exception) in src/types/endpoint_url.rs
- [X] T010 Add TryFrom<String> implementations for all newtypes in src/types/mod.rs
- [X] T011 Update src/types/mod.rs to export Did, ClientId, EndpointUrl
- [X] T012 Update lib.rs to re-export newtypes in public API

**Checkpoint**: Newtypes ready - security validation can now use EndpointUrl

---

## Phase 3: User Story 1 - Security-Critical Fixes (Priority: P0-BLOCKER) 🎯 MVP

**Goal**: Fix CVE-worthy security issues before any public release

**Independent Test**: External security audit passes with zero CRITICAL/HIGH findings

### Implementation for User Story 1

#### SEC-002: Fix Cryptographic RNG

- [X] T013 [US1] Remove `Default` impl from SigningKeyWrapper in src/crypto/dpop.rs
- [X] T014 [US1] Ensure `DPoPKeyPair::generate()` uses OsRng (verify existing) in src/crypto/dpop.rs

#### SEC-003: Fix Zeroization

- [X] T015 [US1] Replace Zeroize impl to properly zero memory (not generate new key) in src/crypto/dpop.rs
- [X] T016 [P] [US1] Add unit test for zeroization behavior in src/crypto/dpop.rs

#### SEC-001: Enforce HTTPS

- [X] T017 [US1] Update config validation to use EndpointUrl newtype in src/config/mod.rs
- [X] T018 [P] [US1] Update SDKConfig.idp_endpoint from String to EndpointUrl in src/config/mod.rs
- [X] T019 [P] [US1] Update SDKConfig.trust_registry_endpoint from Option<String> to Option<EndpointUrl> in src/config/mod.rs
- [X] T020 [US1] Update all policy URL validation to use EndpointUrl in src/policies/base.rs
- [X] T021 [P] [US1] Update URL validation in src/policies/persona.rs
- [X] T022 [P] [US1] Update URL validation in src/policies/age_proof.rs
- [X] T023 [P] [US1] Update URL validation in src/policies/verified_email.rs
- [X] T024 [P] [US1] Update URL validation in src/policies/verified_phone.rs
- [X] T025 [P] [US1] Update URL validation in src/policies/verified_persona.rs
- [X] T026 [P] [US1] Update URL validation in src/policies/employment.rs
- [X] T027 [P] [US1] Update URL validation in src/policies/trust.rs

#### SEC-005: Clock Skew Tolerance

- [X] T028 [US1] Add clock_skew_seconds field to SDKConfig in src/config/mod.rs
- [X] T029 [US1] Implement clock skew tolerance in exp validation in src/client/lastid_client.rs
- [X] T030 [US1] Add nbf (not-before) claim validation with clock skew in src/client/lastid_client.rs
- [X] T031 [P] [US1] Add unit tests for clock skew edge cases in src/client/lastid_client.rs

#### SEC-004: Token Binding Verification

- [X] T032 [US1] Implement cnf.jkt claim extraction and verification in src/auth/token_manager.rs
- [X] T033 [US1] Add verify_token_binding() method to TokenManager in src/auth/token_manager.rs
- [X] T034 [P] [US1] Add unit tests for token binding mismatch scenarios in src/auth/token_manager.rs

#### SEC-006/SEC-007: Additional Security

- [X] T035 [US1] Add exp claim to DPoP proofs (300s max lifetime) in src/crypto/dpop.rs
- [X] T036 [US1] Wrap internal token cache with SecretString in src/auth/token_manager.rs

**Checkpoint**: Security-critical fixes complete - external audit can proceed

---

## Phase 4: User Story 2 - Community Standards (Priority: P1) + User Story 8 - Documentation Accuracy (Priority: P2)

**Goal**: Meet GitHub Community Standards and fix documentation accuracy

**Independent Test**: GitHub Community Standards shows 100% compliance; README examples compile

### Implementation for User Story 2

- [X] T037 [P] [US2] Create CODE_OF_CONDUCT.md using Contributor Covenant 2.1 in repository root
- [X] T038 [P] [US2] Create SECURITY.md with vulnerability disclosure process in repository root
- [X] T039 [US2] Add CI, crates.io, docs.rs, coverage badges to README.md

### Implementation for User Story 8

- [X] T040 [P] [US8] Fix default features documentation to match Cargo.toml in README.md
- [X] T041 [P] [US8] Fix all broken documentation links in README.md
- [X] T042 [P] [US8] Fix npm package name to match published package in README.md
- [X] T043 [US8] Create lastid.toml.example with all documented options in repository root
- [X] T044 [US8] Document MSRV policy (Rust 1.85+) in README.md

**Checkpoint**: Community standards met, documentation accurate

---

## Phase 5: User Story 3 - Rust Idiom Compliance (Priority: P1)

**Goal**: Pass `cargo clippy --all-features -- -D warnings` with zero warnings

**Independent Test**: Clippy passes; r/rust would approve

### Implementation for User Story 3

#### FR-004: Remaining Newtype Adoption

- [X] T045 [US3] Update IssuerInfo.enterprise_did from String to Did in src/trust_registry/types.rs
- [X] T046 [P] [US3] Update IssuerInfo.endpoint_url from String to EndpointUrl in src/trust_registry/types.rs
- [X] T047 [P] [US3] Update VerifiedCredential.subject_did from String to Did in src/types/verified_credential.rs
- [X] T048 [P] [US3] Update VerifiedCredential.issuer_did from String to Did in src/types/verified_credential.rs
- [X] T049 [US3] Update SDKConfig.client_id from String to ClientId in src/config/mod.rs

#### FR-005: Non-Exhaustive Enums (remaining)

- [X] T050 [P] [US3] Add `#[non_exhaustive]` to RequestStatus in src/types/request_status.rs
- [X] T051 [P] [US3] Add `#[non_exhaustive]` to IssuerStatus in src/trust_registry/types.rs
- [X] T052 [P] [US3] Add `#[non_exhaustive]` to TrustLevel in src/policies/trust.rs
- [X] T053 [P] [US3] Add `#[non_exhaustive]` to PersonaField in src/policies/persona.rs
- [X] T054 [P] [US3] Add `#[non_exhaustive]` to EmploymentStatus in src/policies/employment.rs
- [X] T055 [P] [US3] Add `#[non_exhaustive]` to EmploymentField in src/policies/employment.rs
- [X] T056 [P] [US3] Add `#[non_exhaustive]` to AgeComparison in src/policies/age_proof.rs
- [X] T057 [P] [US3] Add `#[non_exhaustive]` to VerificationLevel in src/policies/verified_persona.rs
- [X] T058 [P] [US3] Add `#[non_exhaustive]` to VerifiedPersonaField in src/policies/verified_persona.rs

#### FR-014: Magic Numbers

- [X] T059 [US3] Replace hardcoded timeout 30 with DEFAULT_TIMEOUT_SECONDS in src/http/client.rs
- [X] T060 [P] [US3] Replace hardcoded pool_idle_timeout 30 with constant in src/http/client.rs
- [X] T061 [P] [US3] Replace hardcoded pool_max_idle_per_host 5 with constant in src/http/client.rs
- [X] T062 [P] [US3] Replace hardcoded retry_after 60 with DEFAULT_RETRY_AFTER_SECONDS in src/http/idp_client.rs
- [X] T063 [P] [US3] Replace hardcoded retry_after 60 with constant in src/trust_registry/client.rs

#### FR-006: Clippy Compliance

- [X] T064 [US3] Run `cargo clippy --all-features -- -D warnings` and fix any remaining warnings

**Checkpoint**: Rust idiom compliance verified

---

## Phase 6: User Story 4 - Test Coverage Reality (Priority: P1)

**Goal**: Real test coverage with no `#[ignore]` on security tests

**Independent Test**: All tests pass; zero `#[ignore]` on security-critical tests

### Tests for User Story 4

- [X] T065 [US4] Remove `#[ignore]` from test_verify_presentation_success in tests/poll_and_verify.rs
- [X] T066 [US4] Implement real P-256 key signing in create_mock_sdjwt helper in tests/poll_and_verify.rs
- [X] T067 [US4] Create tests/wasm/mod.rs with wasm_bindgen_test setup
- [X] T068 [P] [US4] Add WASM test for timestamp function (js_sys::Date::now()) in tests/wasm/mod.rs
- [X] T069 [P] [US4] Add WASM test for cache operations (micro-moka) in tests/wasm/mod.rs
- [X] T070 [P] [US4] Add WASM test for WebSocket transport in tests/wasm/mod.rs

### CI Updates for User Story 4

- [X] T071 [US4] Add example compilation step to CI in .github/workflows/ci.yml
- [X] T072 [US4] Fix coverage threshold documentation (80% not 100%) in README.md

**Checkpoint**: Test coverage reflects reality

---

## Phase 7: User Story 5 - Enterprise Network (Priority: P2) + User Story 6 - Integration Pain (Priority: P2)

**Goal**: SDK works in corporate networks with clear error handling

**Independent Test**: SDK operates through HTTP proxy; timeout returns Err()

### Implementation for User Story 5

#### FR-007/FR-008: Network Configuration

- [X] T073 [US5] Create NetworkConfig struct with proxy fields in src/config/network.rs
- [X] T074 [US5] Add connect_timeout_seconds, read_timeout_seconds to NetworkConfig in src/config/network.rs
- [X] T075 [US5] Add pool_idle_timeout_seconds, pool_max_idle_per_host to NetworkConfig in src/config/network.rs
- [X] T076 [US5] Integrate NetworkConfig into SDKConfig in src/config/mod.rs
- [X] T077 [US5] Implement proxy support using reqwest .proxy() in src/http/client.rs
- [X] T078 [US5] Implement granular timeout configuration in src/http/client.rs

### Implementation for User Story 6

#### FR-010: Timeout Return Type

- [X] T079 [US6] Change poll_for_completion timeout from Ok(Timeout) to Err(LastIDError::Timeout) in src/client/lastid_client.rs (native)
- [X] T080 [P] [US6] Change poll_for_completion timeout return in src/client/lastid_client.rs (WASM)

#### FR-011: Correlation IDs

- [X] T081 [US6] Add correlation_id_header field to NetworkConfig in src/config/network.rs
- [X] T082 [US6] Add correlation ID header to all HTTP requests in src/http/idp_client.rs

#### FR-009: Client ID Validation

- [X] T083 [US6] Add early validation that client_id is non-empty in SDKConfig::validate() in src/config/mod.rs

#### FR-012: WebSocket Fallback Logging

- [X] T084 [US6] Add warning log when falling back from WebSocket to polling in src/client/lastid_client.rs

**Checkpoint**: Enterprise network and integration improvements complete

---

## Phase 8: User Story 7 - Simplification (Priority: P2)

**Goal**: Evaluate and simplify over-engineered patterns

**Independent Test**: New developer understands core flow in 30 minutes

### Evaluation Tasks for User Story 7 (No Implementation Required)

- [X] T085 [US7] EVALUATE: Consider changing default feature to include all policies in Cargo.toml
  - **Decision**: Keep current structure. base-policy as default allows minimal builds.
- [X] T086 [US7] EVALUATE: Review type-state pattern necessity for single-field policies in src/policies/base.rs
  - **Decision**: Keep type-state pattern. Provides compile-time safety for required fields.
- [X] T087 [US7] EVALUATE: Assess shared.rs abstraction utility vs direct cfg blocks in src/shared.rs
  - **Decision**: Keep shared.rs. Centralizes platform-specific code, reduces duplication.
- [X] T088 [US7] EVALUATE: Review cache trait hierarchy complexity in src/trust_registry/cache.rs
  - **Decision**: Keep current design. Cache abstraction enables testing and platform flexibility.

**Note**: These tasks are evaluation/decision tasks. Implementation depends on findings.

**Checkpoint**: Simplification opportunities documented

---

## Phase 9: User Story 9 - Observability (Priority: P3) + User Story 10 - Serverless (Priority: P3)

**Goal**: Production monitoring and serverless deployment support

**Independent Test**: Tracing emits structured spans; DPoP keys can be persisted/restored

### Implementation for User Story 9

- [X] T089 [P] [US9] Ensure tracing spans include request_id field in src/client/lastid_client.rs
- [X] T090 [P] [US9] Add cache hit/miss logging when tracing enabled in src/trust_registry/client.rs

### Implementation for User Story 10

- [X] T091 [US10] Add Serialize/Deserialize to DPoPKeyPair for persistence in src/crypto/dpop.rs
- [X] T092 [US10] Add ClientBuilder::with_keypair() method to accept persisted keypair in src/client/builder.rs
- [X] T093 [P] [US10] Add unit tests for keypair serialization roundtrip in src/crypto/dpop.rs

**Checkpoint**: Observability and serverless support complete

---

## Phase 10: Polish & Cross-Cutting Concerns

**Purpose**: Final verification and cleanup

- [X] T094 Run full clippy check: `cargo clippy --all-features -- -D warnings`
- [X] T095 [P] Run cargo audit for security advisories: `cargo audit`
- [X] T096 [P] Run all tests: `cargo test --all-features`
- [X] T097 [P] Run WASM tests: `wasm-pack test --headless --chrome`
- [X] T098 Verify documentation builds: `cargo doc --all-features --no-deps`
- [X] T099 Build all examples: `cargo build --examples --all-features`
- [X] T100 Run coverage check: `cargo llvm-cov --all-features --fail-under 80`

---

## Dependencies & Execution Order

### Phase Dependencies

```
Phase 1 (Setup)
    ↓
Phase 2 (Foundational: Newtypes) ─────────┐
    ↓                                      │
Phase 3 (US1: Security - P0) ◄─────────────┤ (uses EndpointUrl)
    ↓                                      │
Phase 4 (US2+US8: Community/Docs - P1/P2) ─┤ (independent)
    ↓                                      │
Phase 5 (US3: Rust Idioms - P1) ◄──────────┤ (uses newtypes)
    ↓                                      │
Phase 6 (US4: Tests - P1)                  │
    ↓                                      │
Phase 7 (US5+US6: Network/Integration - P2)│ (uses constants)
    ↓                                      │
Phase 8 (US7: Simplification - P2) ────────┘ (evaluation only)
    ↓
Phase 9 (US9+US10: Observability/Serverless - P3)
    ↓
Phase 10 (Polish)
```

### User Story Dependencies

| Story | Depends On | Can Parallelize With |
|-------|------------|---------------------|
| US1 (Security) | Foundational (newtypes) | None (P0 blocker) |
| US2 (Community) | None | US8 |
| US3 (Rust Idioms) | Foundational | US2, US4 |
| US4 (Tests) | US1 (security fixes to test) | US2, US3 |
| US5 (Network) | US3 (constants) | US6 |
| US6 (Integration) | US3 (constants) | US5 |
| US7 (Simplification) | None | US5, US6 |
| US8 (Docs) | None | US2, US3, US4 |
| US9 (Observability) | None | US10 |
| US10 (Serverless) | None | US9 |

### Parallel Opportunities

**Phase 1 (Setup)**:
```
T002, T003, T004, T005, T006 can run in parallel (different files)
```

**Phase 2 (Foundational)**:
```
T008, T009 can run in parallel (different newtype files)
```

**Phase 3 (US1 Security)**:
```
T018, T019 (config fields) in parallel
T021-T027 (policy files) all in parallel
T016, T031, T034 (unit tests) in parallel
```

**Phase 4 (US2+US8)**:
```
T037, T038 (community files) in parallel
T040, T041, T042, T043 (docs) all in parallel
```

**Phase 5 (US3 Rust Idioms)**:
```
T046-T048 (type updates) in parallel
T050-T058 (non_exhaustive) all in parallel
T060-T063 (constants) all in parallel
```

**Phase 6 (US4 Tests)**:
```
T068, T069, T070 (WASM tests) in parallel
```

**Phase 9 (US9+US10)**:
```
T089, T090 (observability) in parallel
T093 (tests) in parallel with T091-T092
```

---

## Parallel Example: Phase 5 (Rust Idioms)

```bash
# Launch all non_exhaustive tasks together:
Task: "[US3] Add #[non_exhaustive] to RequestStatus in src/types/request_status.rs"
Task: "[US3] Add #[non_exhaustive] to IssuerStatus in src/trust_registry/types.rs"
Task: "[US3] Add #[non_exhaustive] to TrustLevel in src/policies/trust.rs"
Task: "[US3] Add #[non_exhaustive] to PersonaField in src/policies/persona.rs"
# ... all 9 tasks in parallel

# Launch all constant replacements together:
Task: "[US3] Replace hardcoded pool_idle_timeout in src/http/client.rs"
Task: "[US3] Replace hardcoded pool_max_idle_per_host in src/http/client.rs"
Task: "[US3] Replace hardcoded retry_after in src/http/idp_client.rs"
Task: "[US3] Replace hardcoded retry_after in src/trust_registry/client.rs"
```

---

## Implementation Strategy

### MVP First (US1 Security Only)

1. Complete Phase 1: Setup (constants, error variants)
2. Complete Phase 2: Foundational (newtypes)
3. Complete Phase 3: US1 Security
4. **STOP and VALIDATE**: Security audit can proceed
5. No public release until US1 complete

### Incremental Delivery

1. Setup + Foundational → Infrastructure ready
2. US1 Security → Security audit can start (P0 MVP!)
3. US2+US8 Community/Docs → GitHub compliant
4. US3 Rust Idioms → Clippy clean
5. US4 Tests → Coverage real
6. US5+US6 Network/Integration → Enterprise ready
7. US7 Simplification → Documented decisions
8. US9+US10 Observability/Serverless → Production ready
9. Polish → Release ready

### Parallel Team Strategy

With 3 developers after Phase 2:

- Developer A: US1 Security (P0 critical path)
- Developer B: US2+US8 Community/Docs (independent)
- Developer C: Begin US3 Rust Idioms (after A finishes US1)

---

## Task Summary

| Phase | User Stories | Task Count | Parallel Tasks |
|-------|--------------|------------|----------------|
| 1. Setup | - | 6 | 5 |
| 2. Foundational | - | 6 | 2 |
| 3. Security | US1 (P0) | 24 | 15 |
| 4. Community/Docs | US2, US8 (P1/P2) | 8 | 6 |
| 5. Rust Idioms | US3 (P1) | 20 | 17 |
| 6. Tests | US4 (P1) | 8 | 3 |
| 7. Network/Integration | US5, US6 (P2) | 12 | 2 |
| 8. Simplification | US7 (P2) | 4 | 0 (eval) |
| 9. Observability/Serverless | US9, US10 (P3) | 5 | 3 |
| 10. Polish | - | 7 | 4 |
| **Total** | | **100** | **57** |

---

## Notes

- [P] tasks = different files, no dependencies on incomplete tasks
- [Story] label maps task to specific user story for traceability
- US1 (Security) is P0-BLOCKER - must complete before public release
- US7 (Simplification) tasks are evaluation-only; implementation separate
- Commit after each task or logical group
- Stop at any checkpoint to validate story independently
