Skip to content

Commit 6e3fc01

Browse files
committed
docs: add ADR for secrecy crate adoption
- Add comprehensive ADR documenting decision to use secrecy crate - Document rationale, consequences, and alternatives considered - Add technical terms (libc, mlock, mprotect, zeroize) to dictionary
1 parent 3d25af9 commit 6e3fc01

File tree

3 files changed

+379
-1
lines changed

3 files changed

+379
-1
lines changed

docs/decisions/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ This directory contains architectural decision records for the Torrust Tracker D
66

77
| Status | Date | Decision | Summary |
88
| ------------- | ---------- | --------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------ |
9+
| ✅ Accepted | 2025-12-17 | [Secrecy Crate for Sensitive Data Handling](./secrecy-crate-for-sensitive-data.md) | Use secrecy crate for type-safe secret handling with memory zeroing |
910
| ✅ Accepted | 2025-12-14 | [Database Configuration Structure in Templates](./database-configuration-structure-in-templates.md) | Expose structured database fields in templates rather than pre-resolved connection strings |
1011
| ✅ Accepted | 2025-12-13 | [Environment Variable Injection in Docker Compose](./environment-variable-injection-in-docker-compose.md) | Use .env file injection instead of hardcoded values for runtime configuration changes |
1112
| ✅ Accepted | 2025-12-11 | [Cloud-Init SSH Port Configuration with Reboot](./cloud-init-ssh-port-reboot.md) | Use cloud-init with reboot pattern to configure custom SSH ports during VM provisioning |
Lines changed: 372 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,372 @@
1+
# Decision: Use Secrecy Crate for Sensitive Data Handling
2+
3+
## Status
4+
5+
Accepted
6+
7+
## Date
8+
9+
2025-12-17
10+
11+
## Context
12+
13+
### The Problem
14+
15+
Sensitive data (API tokens, passwords, database credentials) is currently stored as plain `String` types throughout the codebase. This creates several security and maintainability issues:
16+
17+
1. **Accidental Exposure**: Secrets appear in full when printed with `Debug` formatting, potentially leaking through:
18+
19+
- Debug logs during development
20+
- Error messages and stack traces
21+
- Panic outputs
22+
- Test output and CI logs
23+
24+
2. **No Type-Level Security**: The type system doesn't distinguish between secrets and regular strings:
25+
26+
- No compile-time guarantee that secrets are handled carefully
27+
- Easy to accidentally log or print secret values
28+
- No centralized place to add security enhancements
29+
30+
3. **Difficult Auditing**: Hard to track secret usage:
31+
32+
- Can't easily find all places where secrets are used
33+
- Can't grep for secret-specific types
34+
- No visibility into when/where secrets are exposed
35+
36+
4. **Memory Security Gap**: Secrets remain in memory even after being dropped, potentially accessible through:
37+
- Memory dumps
38+
- Core dumps
39+
- Swap files
40+
- Process memory inspection
41+
42+
### Examples of Current Problems
43+
44+
```rust
45+
// Current problematic approach
46+
#[derive(Debug)]
47+
pub struct HetznerConfig {
48+
pub api_token: String, // Exposed in debug output!
49+
}
50+
51+
// This accidentally logs the token
52+
tracing::debug!("Config: {:?}", config);
53+
// Output: Config: HetznerConfig { api_token: "hf_abc123..." }
54+
55+
pub struct MysqlConfig {
56+
pub password: String, // Visible in error messages!
57+
}
58+
59+
// Error contains password
60+
return Err(format!("Failed to connect to {:?}", config));
61+
```
62+
63+
### Project Requirements
64+
65+
1. **Identification**: Clearly identify where secrets are used in the codebase
66+
2. **Redacted Output**: Prevent accidental exposure through debug/display formatting
67+
3. **Memory Security**: Wipe secrets from memory when no longer needed
68+
4. **Maintainability**: Keep solution simple and well-documented
69+
5. **Standards Compliance**: Follow Rust ecosystem best practices
70+
71+
## Decision
72+
73+
**Adopt the `secrecy` crate** (https://crates.io/crates/secrecy) as the standard solution for handling sensitive data throughout the codebase.
74+
75+
### Implementation Details
76+
77+
**Core Type**: `Secret<T>` from the `secrecy` crate
78+
79+
```rust
80+
// src/shared/secret.rs
81+
pub use secrecy::{ExposeSecret, Secret, SecretString};
82+
use secrecy::SerializableSecret;
83+
84+
// Enable serialization for String secrets (required for config files)
85+
impl SerializableSecret for String {}
86+
87+
// Domain-specific type aliases for clarity
88+
pub type ApiToken = Secret<String>;
89+
pub type Password = Secret<String>;
90+
```
91+
92+
**Usage Pattern**:
93+
94+
```rust
95+
use crate::shared::{ApiToken, ExposeSecret};
96+
97+
#[derive(Debug, Clone, Serialize, Deserialize)]
98+
pub struct HetznerConfig {
99+
pub api_token: ApiToken, // Automatically redacted in Debug
100+
// ...
101+
}
102+
103+
// Creating with secret
104+
let config = HetznerConfig {
105+
api_token: Secret::new("token".to_string()),
106+
};
107+
108+
// Debug output is safe
109+
println!("{:?}", config); // HetznerConfig { api_token: Secret([REDACTED]) }
110+
111+
// Explicit exposure when needed
112+
let token_str = config.api_token.expose_secret();
113+
```
114+
115+
**Locations to Apply**:
116+
117+
1. **Provider Secrets**:
118+
119+
- `HetznerConfig.api_token``ApiToken`
120+
121+
2. **Database Secrets**:
122+
123+
- `MysqlConfig.password``Password`
124+
125+
3. **API Secrets**:
126+
- `HttpApiSection.admin_token``ApiToken`
127+
- `HttpApiConfig.admin_token``ApiToken`
128+
129+
## Rationale
130+
131+
### Why `secrecy` Crate Over Custom Implementation?
132+
133+
1. **Battle-Tested Security**:
134+
135+
- Used by major Rust projects (diesel, sqlx, etc.)
136+
- Audited and maintained by security-conscious community
137+
- Implements best practices from cryptography experts
138+
139+
2. **Memory Zeroing**:
140+
141+
- Uses `zeroize` crate to securely wipe memory on drop
142+
- Prevents secrets from lingering in memory/swap/core dumps
143+
- Hard to implement correctly in custom solution
144+
145+
3. **Industry Standard**:
146+
147+
- De facto standard for secret handling in Rust
148+
- Well-documented with extensive examples
149+
- Future maintainers will recognize the pattern
150+
151+
4. **Minimal Complexity**:
152+
153+
- Single dependency (`secrecy` + transitive `zeroize`)
154+
- Simple API: `Secret::new()` and `expose_secret()`
155+
- Type aliases reduce boilerplate
156+
157+
5. **Future-Proof**:
158+
- If security requirements evolve (audits, compliance), infrastructure is ready
159+
- Can easily add more advanced features if needed
160+
- No need to retrofit memory zeroing later
161+
162+
### Why Not a Custom Type?
163+
164+
**Pros of Custom**:
165+
166+
- Zero dependencies
167+
- Full control
168+
- Simpler initial implementation
169+
170+
**Cons of Custom** (Why we rejected it):
171+
172+
- ❌ No memory zeroing (significant security gap)
173+
- ❌ Need to implement everything ourselves
174+
- ❌ Risk of security mistakes in implementation
175+
- ❌ Reinventing a well-solved problem
176+
- ❌ Future maintainers less likely to understand custom approach
177+
178+
### Why Not a Hybrid Wrapper?
179+
180+
A custom wrapper around `secrecy` would add:
181+
182+
- Extra abstraction layers
183+
- More code to maintain
184+
- Learning curve for contributors
185+
- No significant benefits over direct usage
186+
187+
The `secrecy` API is already simple and well-designed - wrapping it adds unnecessary complexity.
188+
189+
## Consequences
190+
191+
### Positive
192+
193+
**Security Improvements**:
194+
195+
- Secrets automatically redacted in debug output
196+
- Memory securely wiped on drop (via `zeroize`)
197+
- Type-safe secret handling at compile time
198+
- Industry-standard security practices
199+
200+
**Code Quality**:
201+
202+
- Clear identification of all secret values (grep for `Secret<T>`)
203+
- Explicit `expose_secret()` calls visible in code review
204+
- Type aliases improve readability (`ApiToken` vs `String`)
205+
- Consistent pattern across codebase
206+
207+
**Maintainability**:
208+
209+
- Standard solution recognized by Rust developers
210+
- Extensive documentation and examples available
211+
- Community support and updates
212+
- Easy to audit secret usage
213+
214+
**Minimal Overhead**:
215+
216+
- Small dependency (single crate + zeroize)
217+
- No runtime performance impact
218+
- `no_std` compatible (if we ever need it)
219+
- Well-maintained and stable
220+
221+
### Negative
222+
223+
⚠️ **Learning Curve**:
224+
225+
- Contributors need to learn `expose_secret()` pattern
226+
- Must implement `SerializableSecret` marker trait per type
227+
- Slightly more verbose than plain `String`
228+
229+
**Mitigation**: Add examples to `AGENTS.md` and create comprehensive ADR (this document).
230+
231+
⚠️ **Serialization Boilerplate**:
232+
233+
- Need `impl SerializableSecret for String {}` once
234+
- Intentional friction to prevent accidental serialization
235+
236+
**Mitigation**: Single implementation covers all `Secret<String>` uses.
237+
238+
⚠️ **Dependency Addition**:
239+
240+
- Adds `secrecy` (~50KB) and `zeroize` (~20KB) to dependency tree
241+
242+
**Mitigation**: Tiny, stable dependencies with strong security track record.
243+
244+
### Migration Impact
245+
246+
**Affected Modules** (requires updates):
247+
248+
- `src/domain/provider/hetzner.rs` (API token)
249+
- `src/domain/tracker/database/mysql.rs` (password)
250+
- `src/application/command_handlers/create/config/tracker/http_api_section.rs` (admin token)
251+
- `src/domain/tracker/http_api.rs` (admin token)
252+
- All tests using these types
253+
254+
**Breaking Changes**: None (internal refactoring only)
255+
256+
**Timeline**: Estimated 2-3 sprints for complete migration
257+
258+
## Alternatives Considered
259+
260+
### Alternative 1: Custom `Secret<T>` Type
261+
262+
```rust
263+
// Custom implementation
264+
pub struct Secret<T> {
265+
inner: T,
266+
}
267+
268+
impl<T> Debug for Secret<T> {
269+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
270+
write!(f, "Secret([REDACTED])")
271+
}
272+
}
273+
```
274+
275+
**Rejected because**:
276+
277+
- No memory zeroing on drop (major security gap)
278+
- Would need to add `zeroize` dependency anyway
279+
- Reinventing already-solved problem
280+
- More code to maintain and audit
281+
- Less trustworthy than community-vetted solution
282+
283+
### Alternative 2: Manual Conventions
284+
285+
Use comments and documentation to mark secret fields:
286+
287+
```rust
288+
pub struct Config {
289+
/// SECRET: Never log this field
290+
pub api_token: String,
291+
}
292+
```
293+
294+
**Rejected because**:
295+
296+
- No type-safety or compile-time guarantees
297+
- Easy to accidentally violate conventions
298+
- No automatic redaction of debug output
299+
- No memory security
300+
- Impossible to audit automatically
301+
302+
### Alternative 3: `secrets` Crate
303+
304+
Alternative crate with more advanced features (mlock, mprotect).
305+
306+
**Rejected because**:
307+
308+
- Requires `std` and `libc` (not `no_std` compatible)
309+
- Heavier dependency
310+
- More complexity than we currently need
311+
- `secrecy` is more widely adopted
312+
313+
## Related Decisions
314+
315+
- [Error Context Strategy](./error-context-strategy.md) - Errors must not expose secret values
316+
- [Actionable Error Messages](./actionable-error-messages.md) - Error messages must redact secrets
317+
- [Development Principles](../development-principles.md) - Security and observability principles
318+
319+
## References
320+
321+
- **Secrecy Crate**: https://docs.rs/secrecy/latest/secrecy/
322+
- **Zeroize Crate**: https://docs.rs/zeroize/latest/zeroize/
323+
- **Security Best Practices**: https://owasp.org/www-project-secure-coding-practices-quick-reference-guide/
324+
- **Rust API Guidelines**: https://rust-lang.github.io/api-guidelines/
325+
- **Related Issue**: [Secret Type Introduction Refactor Plan](../refactors/plans/secret-type-introduction.md)
326+
327+
## Implementation Notes
328+
329+
### Phase 1: Setup (Priority: P0)
330+
331+
1. Add `secrecy` dependency to `Cargo.toml`
332+
2. Create `src/shared/secret.rs` module
333+
3. Export types and implement `SerializableSecret` for `String`
334+
4. Add type aliases: `ApiToken`, `Password`, `SecretString`
335+
336+
### Phase 2: Provider Secrets (Priority: P1)
337+
338+
1. Update `HetznerConfig.api_token` to use `ApiToken`
339+
2. Update all Hetzner-related tests
340+
3. Verify no secrets in debug output
341+
342+
### Phase 3: Database Secrets (Priority: P2)
343+
344+
1. Update `MysqlConfig.password` to use `Password`
345+
2. Update all MySQL-related tests
346+
3. Verify template rendering works correctly
347+
348+
### Phase 4: API Secrets (Priority: P2)
349+
350+
1. Update `HttpApiSection.admin_token` to use `ApiToken`
351+
2. Update `HttpApiConfig.admin_token` to use `ApiToken`
352+
3. Update all HTTP API tests
353+
354+
### Phase 5: Documentation (Priority: P3)
355+
356+
1. Update `AGENTS.md` with secret handling rule
357+
2. Add examples to module documentation
358+
3. Update contributing guidelines
359+
360+
### Testing Verification
361+
362+
For each phase, verify:
363+
364+
- ✅ All unit tests pass
365+
- ✅ Debug output shows `[REDACTED]` instead of actual values
366+
- ✅ Serialization/deserialization works correctly
367+
- ✅ Error messages don't expose secrets
368+
- ✅ All linters pass (clippy, rustfmt, etc.)
369+
370+
---
371+
372+
**Last Updated**: 2025-12-17

0 commit comments

Comments
 (0)