feat: add post-quantum hybrid KEM + SQLCipher persistence
Feature 1 — Post-Quantum Hybrid KEM (X25519 + ML-KEM-768): - Create hybrid_kem.rs with keygen, encrypt, decrypt + 11 unit tests - Wire format: version(1) | x25519_eph_pk(32) | mlkem_ct(1088) | nonce(12) | ct - Add uploadHybridKey/fetchHybridKey RPCs to node.capnp schema - Server: hybrid key storage in FileBackedStore + RPC handlers - Client: hybrid keypair in StoredState, auto-wrap/unwrap in send/recv/invite/join - demo-group runs full hybrid PQ envelope round-trip Feature 2 — SQLCipher Persistence: - Extract Store trait from FileBackedStore API - Create SqlStore (rusqlite + bundled-sqlcipher) with encrypted-at-rest SQLite - Schema: key_packages, deliveries, hybrid_keys tables with indexes - Server CLI: --store-backend=sql, --db-path, --db-key flags - 5 unit tests for SqlStore (FIFO, round-trip, upsert, channel isolation) Also includes: client lib.rs refactor, auth config, TOML config file support, mdBook documentation, and various cleanups by user. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
269
docs/src/contributing/coding-standards.md
Normal file
269
docs/src/contributing/coding-standards.md
Normal file
@@ -0,0 +1,269 @@
|
||||
# Coding Standards
|
||||
|
||||
This page defines the engineering standards for quicnprotochat. These are
|
||||
non-negotiable -- all code merged into the repository must conform to these
|
||||
rules. The standards exist to ensure that every milestone produces
|
||||
production-ready, auditable, and secure code.
|
||||
|
||||
---
|
||||
|
||||
## Production-Ready Only
|
||||
|
||||
Every deliverable must be complete and functional. The following are prohibited
|
||||
in any merged code:
|
||||
|
||||
- `todo!()` or `unimplemented!()` macros
|
||||
- Stub implementations or placeholder logic
|
||||
- Mock objects in production code paths (mocks are acceptable only in test code)
|
||||
- Commented-out code blocks
|
||||
- `#[allow(unused)]` on production code (acceptable on generated code from
|
||||
Cap'n Proto codegen)
|
||||
|
||||
If a feature is out of scope for the current milestone, it is **explicitly
|
||||
omitted** with a documented reason (in an ADR or code comment explaining why it
|
||||
is deferred), not silently stubbed.
|
||||
|
||||
---
|
||||
|
||||
## YAGNI / KISS / DRY
|
||||
|
||||
- **YAGNI (You Aren't Gonna Need It):** Do not add features, abstractions, or
|
||||
generic type parameters that are not required by the current milestone.
|
||||
- **KISS (Keep It Simple):** Favour clarity over cleverness. A straightforward
|
||||
`match` statement is preferred over a complex trait hierarchy.
|
||||
- **DRY (Don't Repeat Yourself):** Extract shared logic into functions or
|
||||
modules, but do not create abstractions prematurely. Two occurrences is a
|
||||
coincidence; three is a pattern worth extracting.
|
||||
|
||||
---
|
||||
|
||||
## Spec-First Development
|
||||
|
||||
Document the design before implementing it:
|
||||
|
||||
1. **ADR (Architecture Decision Record)** for significant design decisions. ADRs
|
||||
live in `docs/src/design-rationale/` and are referenced from the
|
||||
[Design Rationale](../design-rationale/overview.md) section.
|
||||
2. **Doc comments** on every public API (`///` for items, `//!` for modules).
|
||||
Doc comments must explain:
|
||||
- What the function/type does.
|
||||
- Invariants and preconditions.
|
||||
- Error conditions and what each error variant means.
|
||||
- Examples where the API is non-obvious.
|
||||
|
||||
```rust
|
||||
/// Creates a new MLS group with the given group ID and returns
|
||||
/// the initial `GroupMember` state.
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
/// Returns `GroupError::CryptoBackend` if the MLS crypto provider
|
||||
/// fails to generate the initial key schedule.
|
||||
///
|
||||
/// Returns `GroupError::InvalidGroupId` if `group_id` is empty.
|
||||
pub fn create_group(
|
||||
group_id: &[u8],
|
||||
identity: &IdentityKeypair,
|
||||
) -> Result<GroupMember, GroupError> {
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Security-by-Design
|
||||
|
||||
### Secrets and Key Material
|
||||
|
||||
- All private key material must be wrapped in `Zeroizing<T>` (from the `zeroize`
|
||||
crate) or implement `Zeroize + ZeroizeOnDrop`.
|
||||
- No secret material in log output at any level (`TRACE`, `DEBUG`, `INFO`,
|
||||
`WARN`, `ERROR`).
|
||||
- When logging key-related operations, log only fingerprints (SHA-256 of the
|
||||
public key), never the key bytes themselves.
|
||||
|
||||
### Error Handling
|
||||
|
||||
- No `unwrap()` or `expect()` on cryptographic operations. All crypto errors
|
||||
must be typed and propagated.
|
||||
- Use `thiserror` for library error types (`quicnprotochat-core`,
|
||||
`quicnprotochat-proto`) and `anyhow` for application-level error handling
|
||||
(`quicnprotochat-server`, `quicnprotochat-client`).
|
||||
- `unwrap()` is acceptable only in:
|
||||
- Test code.
|
||||
- Cases where the invariant is provably guaranteed by the type system
|
||||
(e.g., indexing a fixed-size array with a constant).
|
||||
|
||||
### Constant-Time Comparisons
|
||||
|
||||
- Use constant-time comparison (`subtle::ConstantTimeEq` or equivalent) when
|
||||
comparing authentication tokens, key fingerprints, or any value that could
|
||||
be used in a timing side-channel attack.
|
||||
- Never use `==` to compare secrets or tokens in authentication paths.
|
||||
|
||||
### Input Validation
|
||||
|
||||
- Validate all incoming data at the boundary (RPC handler entry point) before
|
||||
passing it to internal logic.
|
||||
- Length checks: group ID (32 bytes), identity key (32 bytes), channel ID
|
||||
(16 bytes), payload (max 5 MB).
|
||||
- Reject unexpected enum variants or unknown wire versions.
|
||||
|
||||
---
|
||||
|
||||
## Containerisation
|
||||
|
||||
- The server runs in Docker. The `Dockerfile` and `docker-compose.yml` must
|
||||
always be kept up to date with the current build.
|
||||
- Multi-stage build: `rust:bookworm` builder stage, `debian:bookworm-slim`
|
||||
runtime stage.
|
||||
- The Docker image must build and run correctly after every merge to the main
|
||||
branch.
|
||||
|
||||
---
|
||||
|
||||
## Dependency Hygiene
|
||||
|
||||
### Pinned Major Versions
|
||||
|
||||
All dependencies are pinned to a major version in `Cargo.toml`. Minor and patch
|
||||
updates are allowed; major version bumps require justification and review.
|
||||
|
||||
### Preferred Ecosystem
|
||||
|
||||
| Domain | Preferred Crate(s) |
|
||||
|--------|-------------------|
|
||||
| Classical crypto (signing) | `ed25519-dalek` |
|
||||
| Classical crypto (key exchange) | `x25519-dalek` |
|
||||
| Noise protocol | `snow` |
|
||||
| MLS | `openmls`, `openmls_rust_crypto` |
|
||||
| Post-quantum KEM | `ml-kem` |
|
||||
| Serialisation / RPC | `capnp`, `capnp-rpc` |
|
||||
| Async runtime | `tokio` |
|
||||
| Zeroisation | `zeroize` |
|
||||
|
||||
Do not introduce new dependencies without justification. In particular:
|
||||
|
||||
- No alternative async runtimes (async-std, smol).
|
||||
- No alternative serialisation formats (protobuf, MessagePack, JSON) for wire
|
||||
protocol use.
|
||||
- No alternative crypto libraries unless the preferred crate lacks required
|
||||
functionality.
|
||||
|
||||
### Dependency Auditing
|
||||
|
||||
- `cargo audit` must pass in CI with no known vulnerabilities.
|
||||
- `cargo deny check` for license compatibility and duplicate detection.
|
||||
|
||||
---
|
||||
|
||||
## Git Standards
|
||||
|
||||
### Signed Commits
|
||||
|
||||
All commits must be GPG-signed. Unsigned commits will be rejected by CI.
|
||||
|
||||
Configure signing:
|
||||
|
||||
```bash
|
||||
git config --global commit.gpgsign true
|
||||
git config --global user.signingkey <YOUR_GPG_KEY_ID>
|
||||
```
|
||||
|
||||
### Conventional Commits
|
||||
|
||||
Commit messages follow the [Conventional Commits](https://www.conventionalcommits.org/)
|
||||
specification:
|
||||
|
||||
| Prefix | Use |
|
||||
|--------|-----|
|
||||
| `feat:` | A new feature or capability |
|
||||
| `fix:` | A bug fix |
|
||||
| `chore:` | Maintenance (deps, CI, config) |
|
||||
| `docs:` | Documentation changes |
|
||||
| `test:` | Adding or updating tests |
|
||||
| `refactor:` | Code restructuring without behaviour change |
|
||||
|
||||
### Commit Message Format
|
||||
|
||||
```
|
||||
<type>: <short description>
|
||||
|
||||
<body: explain WHY, not just WHAT>
|
||||
```
|
||||
|
||||
The body describes the motivation and context for the change, not just a
|
||||
restatement of the diff. Why was this change necessary? What problem does it
|
||||
solve? What alternatives were considered?
|
||||
|
||||
Example:
|
||||
|
||||
```
|
||||
feat: add KeyPackage TTL eviction on fetch
|
||||
|
||||
KeyPackages older than 24 hours are now filtered out at fetch time,
|
||||
preventing stale key material from being used in MLS group additions.
|
||||
|
||||
Background sweep is deferred to M6 (requires persistent storage).
|
||||
```
|
||||
|
||||
### Branch Strategy
|
||||
|
||||
- Feature branches per milestone: `feat/m1-noise-transport`,
|
||||
`feat/m2-keypackage-as`, etc.
|
||||
- Branch names use lowercase with hyphens.
|
||||
- All work happens on feature branches; direct commits to the main branch are
|
||||
prohibited.
|
||||
|
||||
---
|
||||
|
||||
## Code Style
|
||||
|
||||
### Formatting
|
||||
|
||||
- `cargo fmt` with default settings. No custom `rustfmt.toml` overrides.
|
||||
- CI rejects unformatted code.
|
||||
|
||||
### Linting
|
||||
|
||||
- `cargo clippy` with default lints. No `#[allow(clippy::...)]` without a
|
||||
comment explaining why the lint is suppressed.
|
||||
- CI treats clippy warnings as errors.
|
||||
|
||||
### Naming
|
||||
|
||||
- Types: `PascalCase` (Rust convention).
|
||||
- Functions and variables: `snake_case`.
|
||||
- Constants: `SCREAMING_SNAKE_CASE`.
|
||||
- Module names: `snake_case`, matching the file name.
|
||||
|
||||
### Module Organisation
|
||||
|
||||
- One concept per file. A file should not exceed ~500 lines (guideline, not
|
||||
hard rule).
|
||||
- Public API at the top of the file; private helpers at the bottom.
|
||||
- `mod.rs` files are avoided; use `foo.rs` + `foo/` directory when a module
|
||||
has submodules.
|
||||
|
||||
---
|
||||
|
||||
## Review Checklist
|
||||
|
||||
Before presenting any code for review, verify:
|
||||
|
||||
- [ ] No missing error handling (all `Result` values handled).
|
||||
- [ ] No security gaps (secrets zeroized, no secret logging, typed crypto errors).
|
||||
- [ ] No incomplete implementations (no `todo!()`, `unimplemented!()`, stubs).
|
||||
- [ ] No deviation from these standards.
|
||||
- [ ] Doc comments on all public items.
|
||||
- [ ] Tests for all new functionality (see [Testing Strategy](testing.md)).
|
||||
- [ ] `cargo fmt`, `cargo clippy`, and `cargo test --workspace` all pass.
|
||||
|
||||
---
|
||||
|
||||
## Cross-references
|
||||
|
||||
- [Testing Strategy](testing.md) -- test structure and conventions
|
||||
- [Design Rationale](../design-rationale/overview.md) -- ADR index
|
||||
- [Milestones](../roadmap/milestones.md) -- what each milestone delivers
|
||||
- [Production Readiness WBS](../roadmap/production-readiness.md) -- governance and CI requirements
|
||||
Reference in New Issue
Block a user