Skip to content

Commit c66cd2c

Browse files
JstatiaCopilot
Jstatia
and
Copilot
committed
Remove unused deps, migrate once_cell to std, add top-level docs
Dependency cleanup: - Remove parking_lot (declared but never imported) - Remove azure_security_keyvault_certificates (declared but never imported) - Migrate once_cell::sync::Lazy to std::sync::LazyLock across 7 crates Top-level documentation: - native/README.md — root entry point with quick start for Rust/C/C++ - native/CONTRIBUTING.md — development setup, code style, testing, PR checklist - native/docs/FFI-OWNERSHIP.md — Rust-owns/C-borrows rules with diagrams - native/docs/DEPENDENCY-PHILOSOPHY.md — why each dep exists, minimal footprint Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 0b79ebf commit c66cd2c

File tree

26 files changed

+2382
-1063
lines changed

26 files changed

+2382
-1063
lines changed

‎native/CONTRIBUTING.md‎

Lines changed: 370 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,370 @@
1+
<!-- Copyright (c) Microsoft Corporation. Licensed under the MIT License. -->
2+
3+
# Contributing to Native CoseSignTool
4+
5+
Thank you for your interest in improving the native COSE Sign1 SDK. This guide
6+
covers everything you need to build, test, and submit changes to the
7+
`native/` directory.
8+
9+
## Table of Contents
10+
11+
- [Development Setup](#development-setup)
12+
- [Building](#building)
13+
- [Testing](#testing)
14+
- [Coverage Requirements](#coverage-requirements)
15+
- [Code Style](#code-style)
16+
- [Architecture Rules](#architecture-rules)
17+
- [Naming Conventions](#naming-conventions)
18+
- [PR Review Checklist](#pr-review-checklist)
19+
- [Adding a New Extension Pack](#adding-a-new-extension-pack)
20+
- [Adding a New FFI Export](#adding-a-new-ffi-export)
21+
22+
---
23+
24+
## Development Setup
25+
26+
### Required Tools
27+
28+
| Tool | Version | Purpose |
29+
|------|---------|---------|
30+
| **Rust** | stable (edition 2021) | Core implementation |
31+
| **OpenSSL** | 3.0+ | Crypto backend (via `OPENSSL_DIR` or vcpkg) |
32+
| **CMake** | 3.20+ | C/C++ projection builds |
33+
| **C compiler** | C11 (MSVC / GCC / Clang) | C projection tests |
34+
| **C++ compiler** | C++17 (MSVC 2017+ / GCC 7+ / Clang 5+) | C++ projection tests |
35+
| **vcpkg** | Latest | Recommended C/C++ consumption path |
36+
37+
### Optional Tools
38+
39+
| Tool | Purpose |
40+
|------|---------|
41+
| OpenCppCoverage | C/C++ line coverage on Windows |
42+
| cargo-llvm-cov | Rust line coverage (`cargo +nightly llvm-cov`) |
43+
| GTest | C/C++ test framework (fetched automatically by CMake) |
44+
45+
### OpenSSL via vcpkg (recommended on Windows)
46+
47+
```powershell
48+
vcpkg install openssl:x64-windows
49+
$env:OPENSSL_DIR = "C:\vcpkg\installed\x64-windows"
50+
$env:PATH = "$env:OPENSSL_DIR\bin;$env:PATH"
51+
```
52+
53+
### First Build
54+
55+
```powershell
56+
cd native/rust
57+
cargo build --workspace # debug build — verifies toolchain + OpenSSL
58+
cargo test --workspace # run all Rust tests
59+
```
60+
61+
---
62+
63+
## Building
64+
65+
### Rust
66+
67+
```powershell
68+
cd native/rust
69+
cargo build --release --workspace # release build (produces FFI .lib / .dll)
70+
cargo check --workspace # type-check only (faster iteration)
71+
```
72+
73+
### C Projection
74+
75+
```powershell
76+
cd native/rust && cargo build --release --workspace # build FFI libs first
77+
cd native/c
78+
cmake -B build -DBUILD_TESTING=ON
79+
cmake --build build --config Release
80+
```
81+
82+
### C++ Projection
83+
84+
```powershell
85+
cd native/rust && cargo build --release --workspace # build FFI libs first
86+
cd native/c_pp
87+
cmake -B build -DBUILD_TESTING=ON
88+
cmake --build build --config Release
89+
```
90+
91+
### Via vcpkg (builds everything)
92+
93+
```powershell
94+
vcpkg install cosesign1-validation-native[cpp,certificates,mst,signing] `
95+
--overlay-ports=native/vcpkg_ports
96+
```
97+
98+
---
99+
100+
## Testing
101+
102+
### Rust
103+
104+
```powershell
105+
cd native/rust
106+
cargo test --workspace # all tests
107+
cargo test -p cose_sign1_validation # single crate
108+
cargo test -p cose_sign1_certificates -- --nocapture # with stdout
109+
```
110+
111+
### C / C++
112+
113+
```powershell
114+
# After building (see above)
115+
ctest --test-dir native/c/build -C Release
116+
ctest --test-dir native/c_pp/build -C Release
117+
```
118+
119+
### Full Pipeline (build + test + coverage + ASAN)
120+
121+
```powershell
122+
./native/collect-coverage-asan.ps1 -Configuration Debug -MinimumLineCoveragePercent 90
123+
```
124+
125+
This single script:
126+
1. Builds Rust FFI crates
127+
2. Runs C projection tests with coverage + ASAN
128+
3. Runs C++ projection tests with coverage + ASAN
129+
4. Fails if coverage < 90%
130+
131+
### Test Conventions
132+
133+
- **Arrange-Act-Assert** pattern in all tests.
134+
- **Parallel-safe**: no shared mutable state, unique temp file names.
135+
- **Both paths**: every feature needs positive *and* negative test cases.
136+
- **FFI null safety**: every pointer parameter in every FFI function needs a
137+
null-pointer test.
138+
- **Roundtrip tests**: sign → parse → validate for end-to-end confidence.
139+
140+
---
141+
142+
## Coverage Requirements
143+
144+
| Component | Minimum | Tool | Command |
145+
|-----------|---------|------|---------|
146+
| Rust library crates | ≥ 90% line | `cargo llvm-cov` | `cd native/rust && ./collect-coverage.ps1` |
147+
| C projection | ≥ 90% line | OpenCppCoverage | `cd native/c && ./collect-coverage.ps1` |
148+
| C++ projection | ≥ 90% line | OpenCppCoverage | `cd native/c_pp && ./collect-coverage.ps1` |
149+
150+
### What May Be Excluded from Coverage
151+
152+
Only FFI boundary stubs may use `#[cfg_attr(coverage_nightly, coverage(off))]`:
153+
154+
| Allowed | Example |
155+
|---------|---------|
156+
| ✅ FFI panic handlers | `handle_panic()` |
157+
| ✅ ABI version functions | `cose_*_abi_version()` |
158+
| ✅ Free functions | `cose_*_free()` |
159+
| ✅ Error accessors | `cose_last_error_*()` |
160+
161+
### What Must NEVER Be Excluded
162+
163+
- Business logic
164+
- Validation / parsing
165+
- Error handling branches
166+
- Crypto operations
167+
- Builder methods
168+
169+
Every `coverage(off)` annotation must include a comment justifying why the code
170+
is unreachable.
171+
172+
---
173+
174+
## Code Style
175+
176+
### Rust
177+
178+
| Rule | Example |
179+
|------|---------|
180+
| Copyright header on every `.rs` file | `// Copyright (c) Microsoft Corporation.` / `// Licensed under the MIT License.` |
181+
| Manual `Display` + `Error` impls | No `thiserror` in production crates |
182+
| `// SAFETY:` comment on every `unsafe` block | Explains why the operation is sound |
183+
| No `.unwrap()` / `.expect()` in production code | Tests are fine |
184+
| Prefer `.into()` over `.to_string()` for literals | `"message".into()` not `"message".to_string()` |
185+
186+
Full formatting and lint rules are in
187+
[`.editorconfig`](../.editorconfig) and the Cargo workspace `[lints]` table.
188+
189+
### C
190+
191+
| Rule | Example |
192+
|------|---------|
193+
| `extern "C"` guards in every header | `#ifdef __cplusplus` / `extern "C" {` / `#endif` |
194+
| Include guards (`#ifndef`) | `#ifndef COSE_SIGN1_VALIDATION_H` |
195+
| `*const` for borrowed pointers | `const cose_sign1_message_t* msg` |
196+
| `*mut` / non-const for ownership transfer | `cose_sign1_message_t** out_msg` |
197+
198+
### C++
199+
200+
| Rule | Example |
201+
|------|---------|
202+
| Move-only classes | Delete copy ctor + copy assignment |
203+
| Null-check in destructor | `if (handle_) cose_*_free(handle_);` |
204+
| `@see` on copy methods | Point to zero-copy alternative |
205+
| Namespace: `cose::sign1::` | Shared types in `cose::` |
206+
207+
---
208+
209+
## Architecture Rules
210+
211+
### Dependencies Flow DOWN Only
212+
213+
```
214+
Primitives ← Domain Crates ← Extension Packs ← FFI Crates ← C/C++ Headers
215+
```
216+
217+
- **Never** depend upward (e.g., primitives must not depend on validation).
218+
- **Never** depend sideways between extension packs (e.g., certificates must
219+
not depend on MST).
220+
221+
### Single Responsibility
222+
223+
| Layer | Allowed | Not Allowed |
224+
|-------|---------|-------------|
225+
| Primitives | Types, traits, constants | Policy, I/O, network |
226+
| Domain crates | Business logic for one area | Cross-area dependencies |
227+
| Extension packs | Service integration via traits | Direct domain-crate coupling |
228+
| FFI crates | ABI translation only | Business logic |
229+
| C/C++ headers | Inline RAII wrappers | Compiled code |
230+
231+
### External Dependency Rules
232+
233+
1. Every external crate must be listed in `allowed-dependencies.toml`.
234+
2. Prefer `std` over third-party (see [DEPENDENCY-PHILOSOPHY.md](docs/DEPENDENCY-PHILOSOPHY.md)).
235+
3. No proc-macro crates in the core dependency path.
236+
4. Azure SDK dependencies only in extension packs, gated behind Cargo features.
237+
238+
---
239+
240+
## Naming Conventions
241+
242+
### Rust Crate Names
243+
244+
| Pattern | Example | Purpose |
245+
|---------|---------|---------|
246+
| `*_primitives` | `cbor_primitives` | Zero-policy trait crates |
247+
| `cose_sign1_*` | `cose_sign1_signing` | Domain / extension crates |
248+
| `*_ffi` | `cose_sign1_signing_ffi` | FFI projection of parent crate |
249+
| `*_local` | `cose_sign1_certificates_local` | Local/test utility crates |
250+
| `*_test_utils` | `cose_sign1_validation_test_utils` | Shared test infrastructure |
251+
252+
### FFI Function Prefixes
253+
254+
| Prefix | Scope | Example |
255+
|--------|-------|---------|
256+
| `cose_` | Shared COSE types | `cose_headermap_new`, `cose_crypto_signer_free` |
257+
| `cose_sign1_` | Sign1 operations | `cose_sign1_message_parse`, `cose_sign1_builder_sign` |
258+
| `cose_sign1_certificates_` | Certificates pack | `cose_sign1_certificates_trust_policy_builder_*` |
259+
| `cose_sign1_mst_` | MST pack | `cose_sign1_mst_options_new` |
260+
| `cose_sign1_akv_` | AKV pack | `cose_sign1_akv_options_new` |
261+
| `did_x509_` | DID:x509 | `did_x509_parse` |
262+
263+
### C++ Class Names
264+
265+
Classes mirror Rust types in `PascalCase` within namespaces:
266+
267+
| Rust | C++ |
268+
|------|-----|
269+
| `CoseSign1Message` | `cose::sign1::CoseSign1Message` |
270+
| `ValidatorBuilder` | `cose::sign1::ValidatorBuilder` |
271+
| `CoseHeaderMap` | `cose::CoseHeaderMap` |
272+
273+
---
274+
275+
## PR Review Checklist
276+
277+
Every native PR is evaluated on these dimensions. Address each before
278+
requesting review.
279+
280+
### 1. Zero-Copy / No-Allocation
281+
282+
- [ ] No unnecessary `.clone()`, `.to_vec()`, `.to_owned()` on large types
283+
- [ ] FFI handle conversions use bounded lifetimes (`<'a>`), not `'static`
284+
- [ ] C++ accessors return `ByteView` (borrowed), not `std::vector` (copied)
285+
- [ ] `_consume` / `_to_message` variants provided where applicable
286+
287+
### 2. Safety & Correctness
288+
289+
- [ ] Every FFI pointer parameter is null-checked before dereference
290+
- [ ] Every `extern "C"` function is wrapped in `catch_unwind`
291+
- [ ] Every `unsafe` block has a `// SAFETY:` comment
292+
- [ ] Memory ownership documented: who allocates, who frees, which `*_free()`
293+
294+
### 3. API Design
295+
296+
- [ ] Builder patterns are fluent (return `&mut self` or `Self`)
297+
- [ ] Error types use manual `Display + Error` (no `thiserror`)
298+
- [ ] C++ classes are move-only (copy deleted)
299+
- [ ] C headers have `extern "C"` guards
300+
301+
### 4. Test Quality
302+
303+
- [ ] Positive and negative paths covered
304+
- [ ] FFI null-pointer safety tests for every parameter
305+
- [ ] Roundtrip test (sign → parse → validate) if applicable
306+
- [ ] No shared mutable state between tests (parallel-safe)
307+
308+
### 5. Documentation
309+
310+
- [ ] Public Rust APIs have `///` doc comments
311+
- [ ] FFI functions have `# Safety` sections
312+
- [ ] C++ methods have `@see` cross-refs to zero-copy alternatives
313+
- [ ] Module-level `//!` comment in every new `lib.rs`
314+
315+
---
316+
317+
## Adding a New Extension Pack
318+
319+
1. Create the crate structure:
320+
321+
```
322+
extension_packs/my_pack/
323+
├── Cargo.toml
324+
├── src/
325+
│ ├── lib.rs # Module docs + pub use
326+
│ ├── signing/mod.rs # If contributing to signing
327+
│ └── validation/
328+
│ ├── mod.rs
329+
│ ├── trust_pack.rs # impl CoseSign1TrustPack
330+
│ ├── fact_producer.rs # impl TrustFactProducer
331+
│ └── key_resolver.rs # impl SigningKeyResolver (optional)
332+
├── tests/ # Integration tests
333+
└── ffi/
334+
├── Cargo.toml # crate-type = ["staticlib", "cdylib"]
335+
└── src/
336+
├── lib.rs # FFI exports with catch_unwind
337+
├── types.rs # Opaque handle types
338+
└── provider.rs # CBOR provider selection
339+
```
340+
341+
2. Add to workspace `members` in `native/rust/Cargo.toml`.
342+
3. Create C header: `native/c/include/cose/sign1/extension_packs/my_pack.h`
343+
4. Create C++ header: `native/c_pp/include/cose/sign1/extension_packs/my_pack.hpp`
344+
5. Add feature to vcpkg port (`native/vcpkg_ports/`).
345+
6. Add `COSE_HAS_MY_PACK` define to CMake.
346+
347+
---
348+
349+
## Adding a New FFI Export
350+
351+
When you add a public API to a Rust library crate that needs C/C++ access:
352+
353+
1. **Rust FFI**: Add `#[no_mangle] pub extern "C" fn cose_*()` in the FFI crate.
354+
2. **C header**: Add matching declaration in the appropriate `.h` file.
355+
3. **C++ header**: Add RAII wrapper method in the corresponding `.hpp` file.
356+
4. **Null tests**: Add null-pointer safety tests for every pointer parameter.
357+
5. **C/C++ tests**: Add GTest coverage for the new function.
358+
359+
The C/C++ headers are hand-maintained (not auto-generated) — this is
360+
intentional to preserve the header hierarchy and enable C++ RAII patterns. See
361+
[rust/docs/ffi_guide.md](rust/docs/ffi_guide.md) for the rationale.
362+
363+
---
364+
365+
## Questions?
366+
367+
- Architecture questions → [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md)
368+
- Ownership/memory questions → [docs/FFI-OWNERSHIP.md](docs/FFI-OWNERSHIP.md)
369+
- Dependency questions → [docs/DEPENDENCY-PHILOSOPHY.md](docs/DEPENDENCY-PHILOSOPHY.md)
370+
- Rust-specific docs → [rust/docs/](rust/docs/)

0 commit comments

Comments
 (0)