# Quickstart: SDK Review Hardening Implementation

**Branch**: `002-sdk-review-hardening` | **Date**: 2026-01-17

## Overview

This guide provides the implementation sequence for addressing the multi-perspective review findings. Changes are ordered by priority (P0 blockers first) and dependency.

---

## Phase 1: Security Fixes (P0 - BLOCKERS)

### 1.1 Fix Zeroization (SEC-003)

**File**: `src/crypto/dpop.rs`

```rust
// BEFORE (wrong):
impl Zeroize for SigningKeyWrapper {
    fn zeroize(&mut self) {
        self.0 = SigningKey::random(&mut rand::thread_rng());
    }
}

// AFTER (correct):
impl Zeroize for SigningKeyWrapper {
    fn zeroize(&mut self) {
        // Zero the key bytes directly
        // SigningKey's Drop impl also zeroizes, but we want explicit control
        let key_bytes = self.0.to_bytes();
        let mut zeroed = GenericArray::<u8, _>::default();
        zeroed.zeroize(); // Ensure the temp is also zeroed
    }
}
```

**Better approach**: Remove `Zeroize` impl entirely, rely on `SecretBox<SigningKey>` which handles zeroization correctly via the `secrecy` crate.

---

### 1.2 Fix RNG Usage (SEC-002)

**File**: `src/crypto/dpop.rs`

```rust
// REMOVE the Default impl entirely - force explicit generation
// impl Default for SigningKeyWrapper { ... }  // DELETE

// Ensure generate() is the only way to create keys
impl DPoPKeyPair {
    pub fn generate() -> Result<Self, LastIDError> {
        let signing_key = SigningKey::random(&mut OsRng); // Already correct
        // ...
    }
}
```

---

### 1.3 Enforce HTTPS (SEC-001)

**File**: `src/types/endpoint_url.rs` (new)

```rust
impl EndpointUrl {
    pub fn new(value: impl Into<String>) -> Result<Self, LastIDError> {
        let url = value.into();

        // Allow HTTPS always
        if url.starts_with("https://") {
            return Ok(Self(url));
        }

        // Allow HTTP only for localhost (dev mode)
        if url.starts_with("http://localhost") || url.starts_with("http://127.0.0.1") {
            return Ok(Self(url));
        }

        // Reject all other HTTP
        Err(LastIDError::InsecureUrl(url))
    }
}
```

**Update all validation sites**:
- `src/config/mod.rs:163-167`
- `src/policies/base.rs:81-85`
- `src/policies/*.rs` (all 8 policy files)

---

### 1.4 Add Clock Skew Tolerance (SEC-005)

**File**: `src/client/lastid_client.rs`

```rust
// Add to SDKConfig
pub clock_skew_seconds: u64, // default: 60

// Update verification
fn verify_time_claims(
    &self,
    exp: DateTime<Utc>,
    nbf: Option<DateTime<Utc>>,
) -> Result<(), LastIDError> {
    let now = Utc::now();
    let skew = Duration::seconds(self.config.clock_skew_seconds as i64);

    // Check not-before with skew
    if let Some(nbf) = nbf {
        if now + skew < nbf {
            return Err(LastIDError::CredentialNotYetValid { nbf, now });
        }
    }

    // Check expiration with skew
    if now - skew >= exp {
        return Err(LastIDError::CredentialExpired { exp, now });
    }

    Ok(())
}
```

---

### 1.5 Add Token Binding Verification (SEC-004)

**File**: `src/auth/token_manager.rs`

```rust
// When receiving token, verify cnf.jkt matches our key thumbprint
fn verify_token_binding(&self, token: &str) -> Result<(), LastIDError> {
    let claims = decode_jwt_claims(token)?;

    if let Some(cnf) = claims.get("cnf") {
        if let Some(jkt) = cnf.get("jkt").and_then(|v| v.as_str()) {
            let our_thumbprint = self.key_pair.thumbprint()?;
            if jkt != our_thumbprint {
                return Err(LastIDError::TokenBindingMismatch {
                    expected: our_thumbprint,
                    actual: jkt.to_string(),
                });
            }
        }
    }

    Ok(())
}
```

---

## Phase 2: Code Idioms (P1)

### 2.1 Add Newtypes (FR-004)

Create new files:
- `src/types/did.rs`
- `src/types/client_id.rs`
- `src/types/endpoint_url.rs`

Update all usages from `String` to newtypes:
- `SDKConfig.idp_endpoint` -> `EndpointUrl`
- `SDKConfig.client_id` -> `ClientId`
- `IssuerInfo.enterprise_did` -> `Did`
- `VerifiedCredential.subject_did` -> `Did`
- `VerifiedCredential.issuer_did` -> `Did`

---

### 2.2 Add `#[non_exhaustive]` (FR-005)

Add to all public enums:
```rust
#[non_exhaustive]
pub enum LastIDError { ... }

#[non_exhaustive]
pub enum RequestStatus { ... }

// ... all others listed in data-model.md
```

---

### 2.3 Extract Magic Numbers (FR-014)

Create `src/constants.rs` with all named constants (see data-model.md).

Update all hardcoded values:
- `src/http/client.rs:39` -> `DEFAULT_POOL_IDLE_TIMEOUT_SECONDS`
- `src/http/client.rs:40` -> `DEFAULT_POOL_MAX_IDLE_PER_HOST`
- `src/http/idp_client.rs:164` -> `DEFAULT_RETRY_AFTER_SECONDS`
- etc.

---

## Phase 3: Test Coverage (P1)

### 3.1 Enable Signature Verification Test (TR-001)

**File**: `tests/poll_and_verify.rs`

```rust
// Remove #[ignore] and implement real signature test
#[tokio::test]
async fn test_verify_presentation_success() {
    // Generate real P-256 key pair
    let signing_key = SigningKey::random(&mut OsRng);
    let verifying_key = signing_key.verifying_key();

    // Create real SD-JWT with actual signature
    let sd_jwt = create_signed_sdjwt(&signing_key, claims);

    // Set up mock trust registry returning the real public key
    let mock_registry = MockTrustRegistry::new()
        .with_issuer(issuer_did, verifying_key);

    // Verify - should succeed with real signature
    let result = client.verify_presentation(&sd_jwt).await;
    assert!(result.is_ok());
}
```

---

### 3.2 Add WASM Tests (TR-004)

**File**: `tests/wasm/mod.rs` (new)

```rust
#![cfg(target_arch = "wasm32")]

use wasm_bindgen_test::*;

wasm_bindgen_test_configure!(run_in_browser);

#[wasm_bindgen_test]
fn test_wasm_timestamp() {
    // Test js_sys::Date::now() timestamp function
    let ts = current_timestamp_secs().unwrap();
    assert!(ts > 0);
}

#[wasm_bindgen_test]
async fn test_wasm_cache() {
    // Test micro-moka cache operations
    let cache = TrustRegistryCache::new(60);
    cache.insert("did:test:123", mock_issuer_info());
    assert!(cache.get("did:test:123").is_some());
}
```

---

### 3.3 Fix Coverage Threshold Mismatch (TR-002)

**File**: `README.md`

Change:
```markdown
- **100% test coverage** requirement
```

To:
```markdown
- **80% test coverage** requirement (enforced in CI)
```

---

### 3.4 Add Example Compilation to CI (TR-003)

**File**: `.github/workflows/ci.yml`

```yaml
examples:
  name: Examples
  runs-on: ubuntu-latest
  steps:
    - uses: actions/checkout@v4
    - uses: dtolnay/rust-toolchain@stable
    - uses: Swatinem/rust-cache@v2
    - name: Check examples compile
      run: cargo build --examples --all-features
    - name: Test example code
      run: cargo test --examples --all-features
```

---

## Phase 4: Networking (P2)

### 4.1 Add Proxy Support (FR-007)

**File**: `src/http/client.rs`

```rust
impl HttpClient {
    pub fn new(config: &NetworkConfig) -> Result<Self, HttpError> {
        let mut builder = reqwest::Client::builder()
            .user_agent(concat!("lastid-sdk/", env!("CARGO_PKG_VERSION")));

        // Add proxy if configured
        if let Some(proxy_url) = &config.proxy_url {
            let mut proxy = reqwest::Proxy::all(proxy_url)?;

            if let Some(auth) = &config.proxy_auth {
                proxy = match auth {
                    ProxyAuth::Basic { username, password } => {
                        proxy.basic_auth(username, password.expose_secret())
                    }
                    ProxyAuth::Bearer { token } => {
                        // Custom header for bearer
                        proxy
                    }
                };
            }

            builder = builder.proxy(proxy);
        }

        // Add no_proxy hosts
        for host in &config.no_proxy {
            builder = builder.no_proxy(reqwest::NoProxy::from_string(host));
        }

        // ... rest of configuration
    }
}
```

---

### 4.2 Fix Timeout Return Type (FR-010)

**File**: `src/client/lastid_client.rs`

```rust
// BEFORE:
if start.elapsed() >= timeout {
    return Ok(RequestStatus::Timeout { ... }); // WRONG
}

// AFTER:
if start.elapsed() >= timeout {
    return Err(LastIDError::Timeout(start.elapsed().as_secs())); // CORRECT
}
```

---

### 4.3 Add Correlation IDs (FR-011)

**File**: `src/http/idp_client.rs`

```rust
async fn request_credential(&self, ...) -> Result<...> {
    let correlation_id = Uuid::new_v4().to_string();

    let response = self.client
        .post(&url)
        .header("Authorization", format!("DPoP {token}"))
        .header("DPoP", &proof)
        .header(&self.config.network.correlation_id_header, &correlation_id)
        .json(&policy)
        .send()
        .await?;

    // ... log correlation_id with tracing
}
```

---

## Verification Checklist

After implementation, verify:

- [ ] `cargo clippy --all-features -- -D warnings` passes
- [ ] `cargo test --all-features` passes (no `#[ignore]` on security tests)
- [ ] `wasm-pack test --headless --chrome` has actual tests
- [ ] `cargo doc --all-features` has zero warnings
- [ ] All newtypes have `TryFrom<String>` implementations
- [ ] All public enums have `#[non_exhaustive]`
- [ ] No hardcoded magic numbers remain
- [ ] Proxy configuration works with test proxy
- [ ] Timeout returns `Err()` not `Ok(Timeout)`
