feather_reader/
safe_link.rs

1//! The `href` type.
2//!
3//! **This is its own module because of the tuple field.** A private field is
4//! private to the MODULE, and `web.rs` is a single ~13,600-line file holding every
5//! `EntryRow` construction — so while the type lived there,
6//! `SafeLink("javascript:alert(1)")` compiled and rendered verbatim into an
7//! `href`. An adversarial review demonstrated exactly that, five different ways.
8//! Here the field is unreachable from `web.rs`, which is what makes "no bypass"
9//! structural rather than aspirational.
10
11/// A string that is safe to place in an `href`.
12///
13/// **Structural, not procedural — and that distinction is the whole point.** The
14/// saved-record path takes an attacker-controlled URL (any atproto client can
15/// write the record), and Askama escapes HTML metacharacters but NOT schemes, so
16/// `javascript:` survives escaping intact.
17///
18/// The defence used to be "remember to call `net::safe_link` before assigning
19/// this field". A cold review measured what that was worth: deleting the call
20/// left **all 679 tests passing**, because every test either exercised the helper
21/// directly or never rendered this row. The control was real and completely
22/// unprotected.
23///
24/// So the field is no longer a `String`. There is no `From<String>`, no public
25/// member, and neither constructor can carry a foreign URL into an `href`:
26/// [`SafeLink::external`] performs the scheme check itself, and
27/// [`SafeLink::entry`] takes an `i64` and a scope query rather than a string, so
28/// it cannot be handed one.
29pub struct SafeLink(String);
30
31impl SafeLink {
32    /// The reader's own link to a cached entry: `/entries/{id}`, plus the
33    /// list's scope query so paging back stays in the list it came from.
34    ///
35    /// **Takes the id and query separately and builds the path itself**, rather
36    /// than accepting a ready-made `String`. A `fn internal(String)` constructor
37    /// is an unrestricted `String` → `href` conduit sitting one identifier away
38    /// from the single site where attacker-controlled data enters — and a review
39    /// proved it, by typing `internal` where `external` was meant: the whole
40    /// `javascript:` hole came back, compiled clean, and passed every test.
41    ///
42    /// An `i64` and a scope query cannot spell a scheme. The result always
43    /// begins `/entries/`, so it is a path by construction, never a URL.
44    /// `scope_qs` is percent-encoded upstream by `qenc`.
45    pub fn entry(id: i64, scope_qs: &str) -> Self {
46        Self(if scope_qs.is_empty() {
47            format!("/entries/{id}")
48        } else {
49            format!("/entries/{id}?{scope_qs}")
50        })
51    }
52
53    /// Attacker-controlled input. Scheme-checked; an unusable URL yields an
54    /// EMPTY link, which the template renders as a row WITHOUT an anchor rather
55    /// than dropping the row — a dropped row is unremovable, because the un-save
56    /// button lives on it.
57    pub fn external(raw: &str) -> Self {
58        Self(crate::net::safe_link(raw).unwrap_or_default())
59    }
60
61    /// [`SafeLink::external`], for a template that omits the link rather than
62    /// rendering it empty.
63    ///
64    /// The two shapes are not interchangeable, and which one a call site wants
65    /// is decided by whether anything else lives on the link. The saved-record
66    /// row keeps the EMPTY link because dropping the row would take the un-save
67    /// button with it — the record would become unremovable from here. The
68    /// reader view has no such passenger: `entry.html` already renders a
69    /// disabled open-original button for an entry with no URL, so `None`
70    /// selects a path that exists and is styled, and an empty `href` would only
71    /// invent a third state meaning the same thing.
72    ///
73    /// It lives here rather than as `.filter(|l| !l.is_empty())` at the call
74    /// site so that "empty means refused" stays a fact of this module. That is
75    /// the same reason the field is in this file at all.
76    pub fn external_opt(raw: &str) -> Option<Self> {
77        let link = Self::external(raw);
78        (!link.is_empty()).then_some(link)
79    }
80
81    pub fn is_empty(&self) -> bool {
82        self.0.is_empty()
83    }
84}
85
86impl std::fmt::Display for SafeLink {
87    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
88        f.write_str(&self.0)
89    }
90}
feather_reader/safe_link.rs

feather_reader/
safe_link.rs
