3.9 KiB
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 functionstidal/src/ranking/executor/mod.rs-- Signal snapshot populationtidal/src/query/executor/mod.rs-- RETRIEVE pipeline reason taggingtidal/src/query/search/executor/pipeline.rs-- SEARCH pipeline reason taggingtidal/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.