quicproquo/docs/src/contributing/coding-standards.md

Coding Standards

This page defines the engineering standards for quicprochat. 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 prost-generated code)

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 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.

/// 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 or I/O in non-test paths. All crypto errors must be typed and propagated.
• Use thiserror for library error types (quicprochat-core, quicprochat-proto, quicprochat-rpc, quicprochat-sdk) and anyhow for application-level error handling (quicprochat-server, quicprochat-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.
• The builder stage does not install extra system packages for code generation — protobuf-src vendors protoc automatically.

---

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)	Notes
Classical crypto (signing)	ed25519-dalek
Classical crypto (key exchange)	x25519-dalek
OPAQUE authentication	opaque-ke (v4)	Ristretto255 + Argon2
MLS	openmls 0.5, openmls_rust_crypto	RFC 9420
Post-quantum KEM	ml-kem (ML-KEM-768, FIPS 203)
Serialisation / RPC (v2)	prost, prost-build, protobuf-src	Primary wire format
Serialisation (v1 legacy)	capnp, capnp-rpc	Legacy only, not for new code
Async runtime	tokio
QUIC transport	quinn
Middleware	tower
Storage	rusqlite with bundled-sqlcipher
Zeroisation	zeroize
Internal serialisation	bincode	For MLS entities and file-backed store

Do not introduce new dependencies without justification. In particular:

• No alternative async runtimes (async-std, smol).
• No alternative serialisation formats for wire protocol use (new code must use Protobuf via prost; Cap'n Proto is legacy-only).
• 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:

git config --global commit.gpgsign true
git config --global user.signingkey <YOUR_GPG_KEY_ID>

Conventional Commits

Commit messages follow the Conventional Commits 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 --workspace -- -D warnings. No #[allow(clippy::...)] without a comment explaining why the lint is suppressed.
• CI treats clippy warnings as errors (-D warnings).

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).
• cargo fmt, cargo clippy --workspace -- -D warnings, and cargo test --workspace all pass.

---

Cross-references

• Testing Strategy -- test structure and conventions
• Design Rationale -- ADR index
• Milestones -- what each milestone delivers
• Production Readiness WBS -- governance and CI requirements