76 lines
3.9 KiB
Markdown
76 lines
3.9 KiB
Markdown
# Security Audit: Briefing UX & Reason Labels
|
|
|
|
## Scope
|
|
|
|
Reviewed all code changes for the `p1-briefing-ux-reason-labels` feature for security implications including information disclosure, injection attacks, denial of service, and data integrity concerns.
|
|
|
|
## Files Audited
|
|
|
|
- `tidal/src/ranking/reason.rs` -- Core types and helper functions
|
|
- `tidal/src/ranking/executor/mod.rs` -- Signal snapshot population
|
|
- `tidal/src/query/executor/mod.rs` -- RETRIEVE pipeline reason tagging
|
|
- `tidal/src/query/search/executor/pipeline.rs` -- SEARCH pipeline reason tagging
|
|
- `tidal/tests/p1_reason_labels.rs` -- Integration tests
|
|
|
|
## Findings
|
|
|
|
### 1. Information Disclosure via Reason Labels -- LOW RISK
|
|
|
|
**Description**: Reason labels expose internal scoring signals (e.g., which sort mode, which candidate strategy) to API consumers. The `context` HashMap on `ReasonLabel` carries structured data like cohort names and window durations.
|
|
|
|
**Assessment**: This is by design per the spec. The `ReasonCode` enum is a closed set of 19 variants -- no arbitrary string data leaks into reason codes. The `context` HashMap carries only values derived from the schema (window names, cohort names) which are already known to the application layer. No PII, no internal system state, no memory addresses.
|
|
|
|
**Verdict**: ACCEPTABLE. The spec explicitly calls this out in "Source Exposure Rules."
|
|
|
|
### 2. Denial of Service via Reason Accumulation -- NO RISK
|
|
|
|
**Description**: Each candidate accumulates reasons from multiple sources (strategy, sort, cohort, exploration, BM25, ANN, scope). Could an adversarial query cause unbounded reason growth?
|
|
|
|
**Assessment**: Maximum reasons per candidate is bounded:
|
|
- At most 1 strategy reason
|
|
- At most 1 sort reason
|
|
- At most 1 cohort reason
|
|
- At most 1 BM25 reason (search only)
|
|
- At most 1 ANN reason (search only)
|
|
- At most 1 scope reason (search only)
|
|
- At most 1 exploration reason
|
|
|
|
Maximum total: 7 reasons per candidate before filtering. `select_top_reasons()` caps output at `MAX_REASONS_PER_RESULT` (3). No unbounded growth possible.
|
|
|
|
**Verdict**: NO RISK.
|
|
|
|
### 3. Signal Snapshot Data Exposure -- LOW RISK
|
|
|
|
**Description**: `compute_raw_score()` now collects `(signal_name, value)` pairs in the snapshot vector. These are exposed in the `signals` field of `RetrieveResult`.
|
|
|
|
**Assessment**: Signal names come from the schema (application-defined). Values are aggregated scores (velocity, decay score), not raw event data. No user identifiers or timestamps are included in the snapshot. The snapshot only includes signals with values > 0.0, so it cannot be used to probe for the absence of signals.
|
|
|
|
**Verdict**: ACCEPTABLE.
|
|
|
|
### 4. BM25 Score Weight Derivation -- NO RISK
|
|
|
|
**Description**: In the search pipeline, TextRelevance weight is computed as `(bm25_score / 30.0).clamp(0.1, 1.0)`. The constant 30.0 is a normalizer.
|
|
|
|
**Assessment**: The clamp prevents NaN/Inf propagation. Division by a constant cannot panic. The weight is purely informational and does not affect ranking order.
|
|
|
|
**Verdict**: NO RISK.
|
|
|
|
### 5. No New External Inputs -- NO RISK
|
|
|
|
**Description**: The feature adds no new API parameters, no new user-controllable inputs, and no new parsing logic. Reason labels are computed from existing internal state (profile definitions, scoring results, BM25 scores). No user-supplied strings flow into reason codes or context values.
|
|
|
|
**Verdict**: NO RISK.
|
|
|
|
## Summary
|
|
|
|
| Finding | Severity | Verdict |
|
|
|---------|----------|---------|
|
|
| Reason label info disclosure | Low | Acceptable by design |
|
|
| Reason accumulation DoS | None | Bounded at 7 -> capped at 3 |
|
|
| Signal snapshot exposure | Low | Aggregated values only |
|
|
| BM25 weight derivation | None | Clamped, no panic path |
|
|
| No new external inputs | None | Pure internal computation |
|
|
|
|
## Verdict
|
|
|
|
**PASS** -- No security issues identified. The feature adds read-only observability data to existing query responses with no new attack surface. |