Skip to content

Commit 868dace

Browse files
authored
Refactor CLAUDE.md documentation to improve AI code review guidance (#586)
1 parent 2cca3d4 commit 868dace

File tree

5 files changed

+165
-31
lines changed

5 files changed

+165
-31
lines changed

.claude/CLAUDE.md

Lines changed: 96 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,61 +1,125 @@
11
# Bitwarden Internal SDK
22

3-
Rust SDK centralizing business logic. You're reviewing code as a senior Rust engineer mentoring
4-
teammates.
3+
Cross-platform Rust SDK implementing Bitwarden's core business logic.
54

6-
## Client Pattern
5+
**Rust Edition:** The SDK targets the
6+
[2024](https://doc.rust-lang.org/nightly/edition-guide/rust-2024/index.html) edition of Rust.
77

8-
PasswordManagerClient ([bitwarden-pm](crates/bitwarden-pm/src/lib.rs)) wraps
9-
[bitwarden_core::Client](crates/bitwarden-core/src/client/client.rs) and exposes sub-clients:
10-
`auth()`, `vault()`, `crypto()`, `sends()`, `generators()`, `exporters()`.
8+
**Crate documentation**: Before working in any crate, read available documentation: `CLAUDE.md` for
9+
critical rules, `README.md` for architecture, `examples/` for usage patterns, and `tests/` for
10+
integration tests. **Before making changes or reviewing code, review relevant examples and tests for
11+
the specific functionality you're modifying.**
1112

12-
**Lifecycle**
13+
## Architecture Overview
1314

14-
- Init → Lock/Unlock → Logout (drops instance). Memento pattern for state resurrection.
15+
Monorepo crates organized in **four architectural layers**:
1516

16-
**Storage**
17+
### 1. Foundation Layer
1718

18-
- Consuming apps use `HashMap<UserId, PasswordManagerClient>`.
19+
- **bitwarden-crypto**: Cryptographic primitives and protocols, key store for securely working with
20+
keys held in memory.
21+
- **bitwarden-state**: Type-safe Repository pattern for SDK state (client-managed vs SDK-managed)
22+
- **bitwarden-threading**: ThreadBoundRunner for !Send types in WASM/GUI contexts (uses PhantomData
23+
marker)
24+
- **bitwarden-ipc**: Type-safe IPC framework with pluggable encryption/transport
25+
- **bitwarden-error**: Error handling across platforms (basic/flat/full modes via proc macro)
26+
- **bitwarden-encoding**, **bitwarden-uuid**: Encoding and UUID utilities
1927

20-
## Issues necessitating comments
28+
### 2. Core Infrastructure
2129

22-
**Auto-generated code changes**
30+
- **bitwarden-core**: Base Client struct extended by feature crates via extension traits. **DO NOT
31+
add functionality here - use feature crates instead.**
32+
- **bitwarden-api-api**, **bitwarden-api-identity**: Auto-generated API clients (**DO NOT edit -
33+
regenerate from OpenAPI specs**)
2334

24-
- Changes to `bitwarden-api-api/` or `bitwarden-api-identity/` are generally discouraged. These are
25-
auto-generated from swagger specs.
35+
### 3. Feature Implementations
2636

27-
**Secrets in logs/errors**
37+
- **bitwarden-pm**: PasswordManagerClient wrapping core Client, exposes sub-clients: `auth()`,
38+
`vault()`, `crypto()`, `sends()`, `generators()`, `exporters()`
39+
- Lifecycle: Init → Lock/Unlock → Logout (drops instance)
40+
- Storage: Apps use `HashMap<UserId, PasswordManagerClient>`
41+
- **bitwarden-vault**: Vault item models, encryption/decryption and management
42+
- **bitwarden-collections**: Collection models, encryption/decryption and management
43+
- **bitwarden-auth**: Authentication (send access tokens)
44+
- **bitwarden-send**: Encrypted temporary secret sharing
45+
- **bitwarden-generators**: Password/passphrase generators
46+
- **bitwarden-ssh**: SSH key generation/import
47+
- **bitwarden-exporters**: Vault export/import with multiple formats
48+
- **bitwarden-fido**: FIDO2 two-factor authentication
2849

29-
- Do not log keys, passwords, or vault data in logs or error paths. Redact sensitive data.
50+
### 4. Cross-Platform Bindings
3051

31-
**Business logic in WASM**
52+
- **bitwarden-uniffi**: Mobile bindings (Swift/Kotlin) via UniFFI
53+
- Structs: `#[cfg_attr(feature = "uniffi", derive(uniffi::Record))]`
54+
- Enums: `#[cfg_attr(feature = "uniffi", derive(uniffi::Enum))]`
55+
- Include `uniffi::setup_scaffolding!()` in lib.rs
56+
- **bitwarden-wasm-internal**: WebAssembly bindings (**thin bindings only - no business logic**)
57+
- Structs: `#[derive(Serialize, Deserialize)]` with
58+
`#[cfg_attr(feature = "wasm", derive(Tsify), tsify(into_wasm_abi, from_wasm_abi))]`
3259

33-
- `bitwarden-wasm-internal` contains only thin bindings. Move business logic to feature crates.
60+
## Critical Patterns & Rules
3461

35-
**Unsafe without justification**
62+
### Cryptography (bitwarden-crypto)
3663

37-
- Any `unsafe` block needs a comment explaining why it's safe and what invariants are being upheld.
64+
- **DO NOT modify** without careful consideration - backward compatibility is critical
65+
- **KeyStoreContext**: Never hold across await points
66+
- Naming: `derive_` for deterministic key derivation, `make_` for non-deterministic generation
67+
- Use `bitwarden_crypto::safe` module first (password-protected key envelope, data envelope) instead
68+
of more low-level primitives
69+
- IMPORTANT: Use constant time equality checks
70+
- Do not expose low-level / hazmat functions from the crypto crate.
71+
- Do not expose key material from the crypto crate, use key references in the key store instead
3872

39-
**Changes to `bitwarden-crypto/` core functionality**
73+
### State Management (bitwarden-state)
4074

41-
- Generally speaking, this crate should not be modified. Changes need a comment explaining why.
75+
- **Client-managed**: App and SDK share data pool (requires manual setup)
76+
- **SDK-managed**: SDK exclusively handles storage (migration-based, ordering is critical)
77+
- Register types with `register_repository_item!` macro
78+
- Type safety via TypeId-based type erasure with runtime downcast checks
4279

43-
**New crypto algorithms or key derivation**
80+
### Threading (bitwarden-threading)
4481

45-
- Detailed description, review and audit trail required. Document algorithm choice rationale and
46-
test vectors.
82+
- Use ThreadBoundRunner for !Send types (WASM contexts, GUI handles, Rc<T>)
83+
- Pins state to thread via spawn_local, tasks via mpsc channel
84+
- PhantomData<\*const ()> for !Send marker (zero-cost)
4785

48-
**Encryption/decryption modifications**
86+
### Error Handling (bitwarden-error-macro)
4987

50-
- Verify backward compatibility. Existing encrypted data must remain decryptable.
88+
- Three modes: **basic** (string), **flat** (variant), **full** (structure)
89+
- Generates FlatError trait, WASM bindings, TypeScript interfaces, UniFFI errors
90+
- Conditional code generation via cfg! for WASM
5191

52-
**Breaking serialization**
92+
### Security Requirements
5393

54-
- Backward compatibility required. Users must decrypt vaults from older versions.
94+
- **Never log** keys, passwords, or vault data in logs or error paths
95+
- **Redact sensitive data** in all error messages
96+
- **Unsafe blocks** require comments explaining safety and invariants
97+
- **Encryption/decryption changes** must maintain backward compatibility (existing encrypted data
98+
must remain decryptable)
99+
- **Breaking serialization** strongly discouraged - users must decrypt vaults from older versions
55100

56-
**Breaking API changes**
101+
### API Changes
57102

58-
- Document migration path for clients.
103+
- **Breaking changes**: Automated detection via cross-repo workflow (see commit 9574dcc1)
104+
- TypeScript compilation tested against `clients` repo on PR
105+
- Document migration path for clients
106+
107+
## Development Workflow
108+
109+
**Build & Test:**
110+
111+
- `cargo check --all-features --all-targets` - Quick validation
112+
- `cargo test --workspace --all-features` - Full test suite
113+
114+
**Format & Lint:**
115+
116+
- `cargo +nightly fmt --workspace` - Code formatting
117+
- Use `cargo clippy` to lint code and catch common mistakes
118+
119+
**WASM Testing:**
120+
121+
- `cargo test --target wasm32-unknown-unknown --features wasm -p bitwarden-error -p bitwarden-threading -p bitwarden-uuid` -
122+
WASM-specific tests
59123

60124
## References
61125

@@ -66,3 +130,4 @@ PasswordManagerClient ([bitwarden-pm](crates/bitwarden-pm/src/lib.rs)) wraps
66130
- [Code Style](https://contributing.bitwarden.com/contributing/code-style/)
67131
- [Security Whitepaper](https://bitwarden.com/help/bitwarden-security-white-paper/)
68132
- [Security Definitions](https://contributing.bitwarden.com/architecture/security/definitions)
133+
- [Rust 2024 Edition Guide](https://doc.rust-lang.org/edition-guide/rust-2024/)
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# bitwarden-error-macro
2+
3+
Read any available documentation: [README.md](./README.md) for architecture,
4+
[examples/](./examples/) for usage patterns, and [tests/](./tests/) for integration tests.
5+
6+
## Three Modes
7+
8+
Specified via `#[bitwarden_error(mode)]`:
9+
10+
- **basic**: String errors only—converts to JS string via `ToString`
11+
- **flat**: Variant-based—generates `FlatError` trait, TypeScript union types
12+
- **full**: Structure-based—uses `Serialize` + `tsify` for full error details
13+
14+
Generates platform-specific bindings: `From<T> for JsValue` (WASM), TypeScript interfaces,
15+
`uniffi::Error` (mobile).
16+
17+
## Critical Rules
18+
19+
**Requires `thiserror`**: All error enums must derive `#[derive(Error)]` from thiserror crate.
20+
21+
**Conditional generation**: WASM bindings only generated when `wasm` feature enabled—check with
22+
`cfg!(feature = "wasm")`.
23+
24+
**Unsupported**: `full` mode with `export_as` parameter is not supported.
25+
26+
**Debug with `cargo expand`**: If macro output is unclear, use `cargo expand` to inspect generated
27+
code.

crates/bitwarden-ipc/CLAUDE.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# bitwarden-ipc
2+
3+
Read any available documentation: [README.md](./README.md) for architecture,
4+
[examples/](./examples/) for usage patterns, and [tests/](./tests/) for integration tests.
5+
6+
## Critical Rules
7+
8+
**RPC types require serialization**: All `RpcRequest` and response types must implement
9+
`Serialize + DeserializeOwned`.
10+
11+
**Subscribe before sending**: Call `subscribe()` before `send()` to prevent race conditions where
12+
messages arrive before subscription.
13+
14+
**Call `start()` first**: Client must be started via `start()` before subscribing or
15+
`SubscribeError::NotStarted` is returned.
16+
17+
**Handler name collisions**: Registering multiple handlers with the same `RpcRequest::NAME` causes
18+
last registration to silently win—no error is raised.

crates/bitwarden-state/CLAUDE.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# bitwarden-state
2+
3+
Read any available documentation: [README.md](./README.md) for architecture,
4+
[examples/](./examples/) for usage patterns, and [tests/](./tests/) for integration tests.
5+
6+
## Critical Rules
7+
8+
**SDK-managed types require serialization**: Types must implement `Serialize + DeserializeOwned` to
9+
use SDK-managed storage.
10+
11+
**Initialize database before access**: Call `initialize_database()` before accessing SDK-managed
12+
repositories or `get_sdk_managed()` returns `DatabaseNotInitialized` error.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# bitwarden-threading
2+
3+
Read any available documentation: [README.md](./README.md) for architecture,
4+
[examples/](./examples/) for usage patterns, and [tests/](./tests/) for integration tests.
5+
6+
## Critical Rules
7+
8+
**Native requires LocalSet**: `tokio::task::spawn_local` panics without LocalSet context—WASM does
9+
not have this requirement.
10+
11+
**No blocking operations**: Blocking tasks stalls the entire runner since all tasks execute on a
12+
single thread.

0 commit comments

Comments
 (0)