Ask markup and redirect findings what their values hold and where they lead

CHANGELOG.md

@@ -4,6 +4,8 @@ Notable changes to JevGate. Versions follow [Semantic Versioning](https://semver
 
 ## [Unreleased]
 
+- Injection: the follow-up that asks a path finding what its paths can hold now also asks a markup finding what its values hold where they enter the markup (already escaped, percent-encoded or serialized as a URL; typed; the program's own; or raw text) and a redirect finding where its targets can lead (a fixed path such as `/admin` first keeps it on the site; `origin + next` with no slash between them does not). Leaning to harmless values, the finding is a note. On the 38 corpus projects with such findings, vaultwarden's `hibp_breach` (a username percent-encoded before the link), its admin login redirect and shiori's login redirect, all labeled wrong, are notes; the 26 markup and 6 redirect findings labeled right put at most 0.22 and 0.44 on the harmless options and stay. About $0.01 on the corpus.
+
 ## [0.23.0] - 2026-09-27
 
 Three changes to maintainability findings, each from the findings JevGate's own release check got wrong and measured on the corpus with every changed review and consider labeled by hand (a debatable one counting as not right): considers went from 57% to 59% right on the projects used for tuning, from 47% to 51% on the held-out ones and from 24% to 28% on 23 Bend 2 projects never used for tuning; no review changed but one wrong file-organization review. The self-check's baseline is empty, down from four accepted findings. Two changes to security findings come from the reviews JevGate got wrong on vaultwarden, found while preparing a pull request to it.

src/catalog.rs

@@ -292,7 +292,7 @@ pub fn rule_version(key: &str) -> &'static str {
         SHARED_LOGIC => "22",
         TEST_VALUE => "7",
         TEST_REDUNDANCY => "4",
-        INJECTION => "11",
+        INJECTION => "12",
         SENSITIVE_DATA => "8",
         HARDCODED_VALUES => "8",
         UNSAFE_SETTINGS => "6",

src/units/compose.rs

@@ -3,8 +3,8 @@
 use super::{
     Access, Block, Detail, FilePlan, Presence, UnitPlan,
     outcome::{
-        Answers, Outcome, at_most_note, benefit, checks, choice, choice_mass, document_split,
-        logs_found, lowered, noul, open, origin_outcome, rests_on_paths, score, settled_checks,
+        Answers, Outcome, at_most_note, benefit, checks, choice, choice_mass, confirmable,
+        document_split, logs_found, lowered, noul, open, origin_outcome, score, settled_checks,
         several_kind, unit_outcome, value_signals,
     },
     wording::{Wording, comment_reason, comment_wording},
@@ -250,28 +250,44 @@ fn rechecked<'a>(unit: &UnitPlan, judgments: &'a [Judgment]) -> (Outcome, Answer
 }
 
 /// Security units whose finding a confirm Choice of their own follows, not
-/// yet asked: an injection finding that rests on a path (what its paths can
-/// hold) and a sensitive-data finding its log checks raised (when the log
-/// line runs).
+/// yet asked: an injection finding whose one concern is a path (what its
+/// paths can hold), or markup or a redirect unless its values are asked
+/// already (what they hold, where they lead), and a sensitive-data finding
+/// its log checks raised (when the log line runs).
 pub fn unconfirmed_units(plan: &FilePlan, judgments: &[Judgment]) -> BTreeSet<String> {
     plan.units
         .iter()
         .filter(|u| u.presence == Presence::Judged)
         .filter(|u| answers(judgments, &u.id, Pass::Locate).is_empty())
         .filter(|u| {
-            let Detail::Security { paths, logging, .. } = &u.detail else {
+            let Detail::Security {
+                checked, logging, ..
+            } = &u.detail
+            else {
                 return false;
             };
             let (outcome, resolved) = resolved(u, judgments);
             let get = |q: &str| resolved.get(q).copied();
+            let kind = confirmable(&get);
             matches!(outcome, Outcome::Review(_) | Outcome::Consider(_))
-                && (paths.is_some() && rests_on_paths(&get)
+                && (checked.is_some()
+                    && (kind == Some("path") || kind.is_some() && !values_due(outcome, &resolved))
                     || logging.is_some() && logs_found(&get))
         })
         .map(|u| u.id.clone())
         .collect()
 }
 
+/// Whether an injection outcome calls for what its values can hold: a
+/// consider that rests on the function's parameters, its origin not
+/// another party.
+fn values_due(outcome: Outcome, resolved: &Answers<'_>) -> bool {
+    matches!(outcome, Outcome::Consider(_))
+        && !resolved
+            .get("origin")
+            .is_some_and(|a| matches!(origin_outcome(a), Outcome::Review(_)))
+}
+
 /// Functions whose split question raised a review or consider, and whose
 /// block has not been located yet; hardcoded-value functions raised to a
 /// review or consider whose value has not been named yet; and the other
@@ -308,11 +324,8 @@ fn locate_due(unit: &UnitPlan, judgments: &[Judgment]) -> bool {
         Detail::Security {
             confirm: Some(_), ..
         } => {
-            matches!(outcome, Outcome::Consider(_))
-                && !resolved
-                    .get("origin")
-                    .is_some_and(|a| matches!(origin_outcome(a), Outcome::Review(_)))
-                && !rests_on_paths(&|q| resolved.get(q).copied())
+            values_due(outcome, &resolved)
+                && confirmable(&|q| resolved.get(q).copied()) != Some("path")
         }
         Detail::Function {
             locate: Some(_), ..

src/units/follow_ups.rs

@@ -16,7 +16,9 @@ pub fn locates(plan: &Plan, files: &[FileResult]) -> Vec<Planned> {
         files,
         compose::unconfirmed_units,
         |unit| match &unit.detail {
-            Detail::Security { paths, logging, .. } => paths.as_ref().or(logging.as_ref()),
+            Detail::Security {
+                checked, logging, ..
+            } => checked.as_ref().or(logging.as_ref()),
             _ => None,
         },
     );

src/units/mod.rs

@@ -193,9 +193,10 @@ pub enum Detail {
         /// For injection, what the values it places can hold, asked only
         /// after a consider that rests on its parameters.
         confirm: Option<FollowUp>,
-        /// For injection, what the variable parts of its file paths can
-        /// hold, asked only after a finding that rests on a path.
-        paths: Option<FollowUp>,
+        /// For injection, what the values of a path, markup or redirect
+        /// finding can hold or where they lead, asked only after a finding
+        /// whose one concern is one of those.
+        checked: Option<FollowUp>,
         /// For sensitive data, when its log line runs, asked only after a
         /// finding its log checks raised.
         logging: Option<FollowUp>,

src/units/outcome/injection.rs

@@ -62,31 +62,54 @@ pub(in crate::units) fn injection_outcome<'a>(
     };
     Some(match by_origin(origin, &found, get) {
         Outcome::Consider(p) if program_values(get) => Outcome::Note(p),
-        Outcome::Review(p) | Outcome::Consider(p) if confined_paths(get) => Outcome::Note(p),
+        Outcome::Review(p) | Outcome::Consider(p) if harmless(get).is_some() => Outcome::Note(p),
         outcome => outcome,
     })
 }
 
-/// Whether every injection check that found a variable placed unhandled is
-/// the path check: such a finding is asked what its paths can hold.
-pub(in crate::units) fn rests_on_paths<'a>(get: &impl Fn(&str) -> Option<&'a Answer>) -> bool {
+/// The kinds of injection whose values a confirm Choice asks about after
+/// the finding, with its question and the options that make it a note.
+const CONFIRMED: [(&str, &str, &[&str]); 3] = [
+    ("path", "paths", &questions::CONFINED_PATHS),
+    ("markup", "markup_values", &questions::HARMLESS_MARKUP),
+    ("redirect", "redirect_reach", &questions::OWN_SITE),
+];
+
+/// The one kind of injection a finding rests on, when every check that
+/// found a variable placed unhandled is that kind and a confirm Choice asks
+/// about it.
+pub(in crate::units) fn confirmable<'a>(
+    get: &impl Fn(&str) -> Option<&'a Answer>,
+) -> Option<&'static str> {
     let found = found_injections(get);
-    !found.is_empty() && found.iter().all(|id| *id == "path")
+    let first = *found.first()?;
+    CONFIRMED
+        .iter()
+        .find(|(kind, ..)| *kind == first && found.iter().all(|id| id == kind))
+        .map(|(kind, ..)| *kind)
 }
 
-/// Whether a path finding's paths, asked after it, lean toward names that
-/// stay inside their directory, the program's own or the local user's: a
-/// route parameter parsed as a UUID or as Rocket's `PathBuf`, a base name or
-/// a checked id. Such a finding is a note. On the corpus, the 5 path
-/// findings labeled right (request parameters and uploaded names joined to
-/// a directory) answered another party's input at 0.96 or more, while
-/// vaultwarden's 4 wrong ones on typed Rocket route parameters leaned to
-/// confined names at 0.67 to 0.78; none reached the threshold, as a type's
-/// parsing is shown only by its derive list.
-pub(in crate::units) fn confined_paths<'a>(get: &impl Fn(&str) -> Option<&'a Answer>) -> bool {
-    rests_on_paths(get)
-        && choice_mass(get("paths"), &questions::CONFINED_PATHS)
-            .is_some_and(|p| probability_at_least(p, LEADING_PROBABILITY))
+/// The kind of a finding whose confirm Choice leans toward values that can
+/// do no harm there: paths that stay inside their directory (a route
+/// parameter parsed as a UUID or as Rocket's `PathBuf`, a base name, a
+/// checked id), markup values already escaped or encoded, or redirect
+/// targets that stay on the site. Such a finding is a note. On the corpus,
+/// the 5 path findings labeled right answered another party's input at 0.96
+/// or more, while vaultwarden's 4 wrong ones on typed Rocket route
+/// parameters leaned to confined names at 0.65 to 0.84, as a type's parsing
+/// is shown only by its derive list. The 26 markup findings labeled right
+/// put at most 0.22 on values that cannot open a tag, and vaultwarden's
+/// percent-encoded username 0.56; the 6 redirect findings labeled right put
+/// at most 0.44 on staying on the site, and vaultwarden's admin path and
+/// shiori's login page 0.68 and 0.58.
+pub(in crate::units) fn harmless<'a>(
+    get: &impl Fn(&str) -> Option<&'a Answer>,
+) -> Option<&'static str> {
+    let kind = confirmable(get)?;
+    let (_, question, clears) = CONFIRMED.iter().find(|(k, ..)| *k == kind)?;
+    choice_mass(get(question), clears)
+        .is_some_and(|p| probability_at_least(p, LEADING_PROBABILITY))
+        .then_some(kind)
 }
 
 /// Whether what a consider's values can hold, asked after it, leans toward

src/units/outcome/mod.rs

@@ -31,7 +31,7 @@ pub(super) use exposure::{
     settings_module_outcome,
 };
 pub(super) use injection::{
-    RESOURCE_CHECKS, confined_paths, injection_outcome, origin_outcome, rests_on_paths,
+    RESOURCE_CHECKS, confirmable, harmless, injection_outcome, origin_outcome,
 };
 pub(super) use maintainability::{
     benign_key, function_outcome, organization_outcome, several_kind, shared_outcome,

src/units/questions/security.rs

@@ -267,6 +267,72 @@ pub fn injection_paths(code: &str, callers: bool, types: bool) -> Value {
 /// The options of `injection_paths` that keep a path inside its directory.
 pub const CONFINED_PATHS: [&str; 3] = ["confined", "own", "local"];
 
+/// What a markup finding's values hold where they enter the markup, asked
+/// only after a finding whose one concern is markup. vaultwarden's
+/// `hibp_breach` percent-encodes the username before it builds the link,
+/// oak's examples write a URL object whose serialization percent-encodes
+/// `<` and `>`, and a JSP page runs its own `esc()` first: the markup check
+/// reads a variable joined into HTML, whatever it was turned into before.
+pub fn markup_values(code: &str, callers: bool) -> Value {
+    let (by_callers, note) = if callers {
+        (
+            " or by the functions in `callers`",
+            format!("{CALLERS} {EVIDENCE}"),
+        )
+    } else {
+        ("", EVIDENCE.to_string())
+    };
+    json!({
+        "type": "choice",
+        "instructions": {
+            "question": format!("What do the values that `{code}` places into HTML or SVG markup hold where they enter it?"),
+            "note": note,
+        },
+        "criteria": {
+            "encoded": format!("Text already escaped for HTML, percent-encoded or serialized as a URL before it enters the markup, in this code{by_callers}, so it cannot hold `<`, `>`, `&` or quotes."),
+            "typed": "Numbers, dates, booleans or ids, or names chosen from a fixed list.",
+            "own": "Text the program writes itself or reads from its configuration.",
+            "raw": "Text as another party or a caller wrote it, which can hold `<`, `>`, `&` or quotes.",
+            "unknown": "Values whose origin or handling is not shown.",
+        },
+    })
+}
+
+/// The options of `markup_values` that cannot open a tag or attribute.
+pub const HARMLESS_MARKUP: [&str; 3] = ["encoded", "typed", "own"];
+
+/// Where a redirect finding's targets can lead, asked only after a finding
+/// whose one concern is a redirect. vaultwarden's admin login redirects to
+/// its admin path followed by the form's value, and shiori's to its login
+/// page with the current path as a query value: a fixed path before the
+/// variable keeps the target on the site, which the redirect check does not
+/// ask. Offered "its own origin and a slash" without the form written out,
+/// chatbot-ui's `requestUrl.origin + next` read as staying on the site at
+/// 0.63, though `next=@evil.com` leaves it.
+pub fn redirect_reach(code: &str, callers: bool) -> Value {
+    let note = if callers {
+        format!("{CALLERS} {EVIDENCE}")
+    } else {
+        EVIDENCE.to_string()
+    };
+    json!({
+        "type": "choice",
+        "instructions": {
+            "question": format!("Where can the targets that `{code}` redirects clients to lead?"),
+            "note": note,
+        },
+        "criteria": {
+            "own_site": "Only to the program's own site: every target starts with a fixed path written in the code, such as `/admin` or `/login?next=`, so it begins with one slash and a path; or with the program's own origin followed by a slash written in the code; variables only follow that fixed part or fill its query string.",
+            "checked": "Only where a check allows: the target is compared with an allowed list of hosts or checked to be a path on the site before the redirect.",
+            "anywhere": "Anywhere a variable says: a variable starts the target, or directly follows the program's own origin or a host with no slash written between them, as in `origin + next`, where `@evil.com` or `.evil.com` in the variable names another host.",
+            "none": "It does not redirect.",
+        },
+    })
+}
+
+/// The options of `redirect_reach` that keep a redirect on the site.
+pub const OWN_SITE: [&str; 3] = ["own_site", "checked", "none"];
+
 /// When a logging finding's log line runs, asked only for a sensitive-data
 /// finding raised by its log checks. vaultwarden logs SSO tokens inside
 /// `if CONFIG.sso_debug_tokens()`, a setting off by default and documented

src/units/security.rs

@@ -354,8 +354,8 @@ fn push_unit(
     let confirm = (rule == INJECTION)
         .then(|| confirm(file, subject, id))
         .flatten();
-    let paths = (rule == INJECTION)
-        .then(|| confirm_paths(file, subject, id))
+    let checked = (rule == INJECTION)
+        .then(|| confirm_checks(file, subject, id))
         .flatten();
     let logging = (rule == SENSITIVE_DATA)
         .then(|| confirm_logging(file, subject, id))
@@ -380,7 +380,7 @@ fn push_unit(
             trace: trace.map(Into::into),
             settles,
             confirm: confirm.map(Into::into),
-            paths: paths.map(Into::into),
+            checked: checked.map(Into::into),
             logging: logging.map(Into::into),
             django: subject.django,
             test_path: subject.test_path,
@@ -428,15 +428,15 @@ fn send(
                     trace,
                     settles,
                     confirm,
-                    paths,
+                    checked,
                     logging,
                     ..
                 } = &mut unit.detail
                 {
                     *trace = None;
                     settles.clear();
                     *confirm = None;
-                    *paths = None;
+                    *checked = None;
                     *logging = None;
                 }
             }
@@ -811,28 +811,28 @@ fn confirm(file: &FileContext<'_>, subject: &Subject<'_>, id: &str) -> Option<(V
     file.budget.fits(&request).then_some((request, asked))
 }
 
-/// What the variable parts of the paths a path finding rests on can hold,
-/// asked only after such a finding: the function, the functions that call
-/// it and the project's types its parameters name.
-fn confirm_paths(
+/// What the values of a path, markup or redirect finding can hold or where
+/// they lead, asked only after a finding whose one concern is one of those;
+/// only the question of its kind is read. The function, the functions that
+/// call it and the project's types its parameters name.
+fn confirm_checks(
     file: &FileContext<'_>,
     subject: &Subject<'_>,
     id: &str,
 ) -> Option<(Value, Asked)> {
     let code = subject.code();
+    let callers = !subject.callers.is_empty();
     let mut questions = Questions::default();
-    questions.ask(
-        "paths".into(),
-        questions::injection_paths(
-            &code,
-            !subject.callers.is_empty(),
-            !subject.types.is_empty(),
+    for (question, body) in [
+        (
+            "paths",
+            questions::injection_paths(&code, callers, !subject.types.is_empty()),
         ),
-        id,
-        INJECTION,
-        "paths",
-        Pass::Locate,
-    );
+        ("markup_values", questions::markup_values(&code, callers)),
+        ("redirect_reach", questions::redirect_reach(&code, callers)),
+    ] {
+        questions.ask(question.into(), body, id, INJECTION, question, Pass::Locate);
+    }
     let mut state = with_callers(file, subject);
     if !subject.types.is_empty() {
         state["types_named_in_parameters"] = json!(subject.types);

src/units/tests/nextjs.rs

@@ -64,6 +64,10 @@ fn an_unchecked_redirect_target_is_an_open_redirect_review() {
         ("resource", noul_at(0.95)),
         ("redirect", noul_at(0.95)),
         ("origin", spread(0.0, 0.05, 0.95)),
+        (
+            "redirect_reach",
+            choice_of("anywhere", &["anywhere", "checked", "none", "own_site"]),
+        ),
     ];
     let report = run(&project, &options, &mut eval);
     let file = report