From 57b3901f54f0bbaf8d065ad6e2065df70d8ee009 Mon Sep 17 00:00:00 2001 From: Tauan BF <11513929+tauanbinato@users.noreply.github.com> Date: Sat, 26 Sep 2026 18:14:30 -0300 Subject: [PATCH 1/9] Ask an injection consider resting on its parameters what its values can hold A consider that says "a caller passing outside input would make it exploitable" is asked, with the functions that call it, what the values it places can hold: text the program fixes, values it creates or a local tool's own arguments make it a note; text another party can set, or callers not shown, keep it. The recheck's origin question answered "the function's parameters" at 0.9 even when every caller passed a literal. Labeled such considers went from 30 right and 51 wrong to 30 and 32. --- CHANGELOG.md | 1 + docs/classification-cascade.md | 9 ++++- site/src/how-it-works.md | 9 ++++- src/catalog.rs | 2 +- src/units/compose.rs | 14 +++++++- src/units/follow_ups.rs | 7 ++-- src/units/mod.rs | 3 ++ src/units/outcome/injection.rs | 14 +++++++- src/units/questions/security.rs | 40 +++++++++++++++++++++++ src/units/security.rs | 49 ++++++++++++++++++++++++++-- src/units/tests/security.rs | 58 ++++++++++++++++++++++++++++++++- 11 files changed, 195 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebd2ab7..91201a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ Measured on 103 pinned projects (24 new open-source ones of kinds not tried befo - Documentation: Claude Code skills, commands and subagent definitions (Markdown under `.claude/skills`, `.claude/commands` and `.claude/agents`) are project documentation, checked for stale paths, repetition and size; a session loads only their descriptions, so they do not count toward what loads at its start. One project's skill cited documentation paths a rename had removed, which nothing reported. A path holding a `$` placeholder, such as a command's `.kiro/specs/$1/spec.json`, names no file. - Project templates: a file under a directory whose name holds a `{{ … }}` placeholder, as a cookiecutter template's `{{cookiecutter.project_slug}}` does, is parsed without its Jinja tags (statements and comments blanked, each placeholder read as a name of the same length, so lines stay the file's) and judged; the evidence keeps the tags. 31 of cookiecutter-django's Python and JavaScript files, the generated application's settings, models, views and tests, were skipped for syntax errors and are judged; its Celery settings turning off Redis certificate checks (`ssl.CERT_NONE`) are a review. - Tests: a test said to assert internal details is asked, with the bodies of the functions it calls, what its assertions read: results, state the program shows or acts on next, or effects a caller observes clear the consider; stored input or calls between the program's own functions keep it. Asked of the test and the signatures it calls, the check read a debug panel's recorded queries (`panel._queries`, which the panel renders), Devise's documented hooks and an app's state after an action as internals: 49 of 66 such considers labeled on the corpus were wrong. A test that reads members through reflection or a cast to `any` keeps its consider without being asked. Labeled tests/value considers went from 29% to 77% right (20 right and 6 wrong, against 22 and 55), and on held-out projects from 2 right and 13 wrong to 1 wrong. About $0.006 of follow-ups on the corpus. +- Injection: a consider that rests on the function's parameters ("a caller passing outside input would make it exploitable") is asked, with the functions that call it, what the values it places can hold. It becomes a note when text the program fixes (literals its callers pass, numbers, names from a fixed list), values it creates or a local tool's own arguments lead; text another party can set, or callers that are not shown, keep it. Asked where the values come from, the recheck answered "the function's parameters" at 0.9 even for a Rust helper whose four callers pass literal SQL fragments. On the labeled projects, such considers went from 30 right and 51 wrong to 30 right and 32 wrong. Only these follow-ups are asked. - Tests: Deno tests are test cases, in each of their forms: `Deno.test("name", fn)`, `Deno.test({ name: "name", fn() {…} })` and `Deno.test(function name() {…})`, with `.only` and `.ignore`. oak writes its 266 tests in the object form, and none of them was judged: its test files got a file-purpose request each and the test rules found nothing to ask. Only Deno projects' requests change. ## [0.20.0] - 2026-09-26 diff --git a/docs/classification-cascade.md b/docs/classification-cascade.md index 47ec914..ce2447d 100644 --- a/docs/classification-cascade.md +++ b/docs/classification-cascade.md @@ -202,7 +202,14 @@ signatures, or one candidate pair. `eval` of model output; a check per kind decides and names the kind. An injection whose origin stays unclear or is the function's parameters is asked its origin and checks again with up to three callers; its answer - replaces the traced one unless only the traced one is decisive. + replaces the traced one unless only the traced one is decisive. A consider + that still rests on the function's parameters is asked, with its callers, + what the values it places can hold: text the program fixes (literals its + callers pass, numbers, names from a fixed list), values it creates itself, + or a local tool's own arguments make it a note; text another party can set, + or callers not shown, keep it. Asked where the values come from, the + recheck answered "the function's parameters" at 0.9 even for a helper + whose every caller passed a literal SQL fragment. The markup check names text shown as a JSX child and CSS values or class names as escaped or inert; sending where each built string goes did not settle React units, the examples did. A sensitive-data trace lists the diff --git a/site/src/how-it-works.md b/site/src/how-it-works.md index 55df00b..2663c47 100644 --- a/site/src/how-it-works.md +++ b/site/src/how-it-works.md @@ -220,7 +220,14 @@ signatures, or one candidate pair. `eval` of model output; a check per kind decides and names the kind. An injection whose origin stays unclear or is the function's parameters is asked its origin and checks again with up to three callers; its answer - replaces the traced one unless only the traced one is decisive. + replaces the traced one unless only the traced one is decisive. A consider + that still rests on the function's parameters is asked, with its callers, + what the values it places can hold: text the program fixes (literals its + callers pass, numbers, names from a fixed list), values it creates itself, + or a local tool's own arguments make it a note; text another party can set, + or callers not shown, keep it. Asked where the values come from, the + recheck answered "the function's parameters" at 0.9 even for a helper + whose every caller passed a literal SQL fragment. The markup check names text shown as a JSX child and CSS values or class names as escaped or inert; sending where each built string goes did not settle React units, the examples did. A sensitive-data trace lists the diff --git a/src/catalog.rs b/src/catalog.rs index 01c3b77..a1349c0 100644 --- a/src/catalog.rs +++ b/src/catalog.rs @@ -277,7 +277,7 @@ pub fn rule_version(key: &str) -> &'static str { SHARED_LOGIC => "20", TEST_VALUE => "6", TEST_REDUNDANCY => "4", - INJECTION => "8", + INJECTION => "9", SENSITIVE_DATA => "7", HARDCODED_VALUES | UNSAFE_SETTINGS => "5", AGENT_CONTEXT => "3", diff --git a/src/units/compose.rs b/src/units/compose.rs index 19f61eb..5c2f747 100644 --- a/src/units/compose.rs +++ b/src/units/compose.rs @@ -59,8 +59,10 @@ fn security_answers<'a>(unit: &UnitPlan, judgments: &'a [Judgment]) -> Answers<' merged.insert(question, answer); } } - // The settle answers sit beside the checks they settle, under their own names. + // The settle answers sit beside the checks they settle, under their own + // names, and so does what an injection consider's values can hold. merged.extend(answers(judgments, &unit.id, Pass::Settle)); + merged.extend(answers(judgments, &unit.id, Pass::Locate)); merged } @@ -248,6 +250,16 @@ pub fn unlocated_units(plan: &FilePlan, judgments: &[Judgment]) -> BTreeSet matches!(outcome, Outcome::Review(_)), // Only the internal-details check raises a test's consider. Detail::Test { confirm: Some(_) } => matches!(outcome, Outcome::Consider(_)), + // An injection consider rests on the function's parameters + // unless its origin was another party. + Detail::Security { + confirm: Some(_), .. + } => { + matches!(outcome, Outcome::Consider(_)) + && !resolved + .get("origin") + .is_some_and(|a| matches!(origin_outcome(a), Outcome::Review(_))) + } Detail::Function { locate: Some(_), .. } => raised(resolved.get("split").map(|a| benefit(a))), diff --git a/src/units/follow_ups.rs b/src/units/follow_ups.rs index 196f95c..804d922 100644 --- a/src/units/follow_ups.rs +++ b/src/units/follow_ups.rs @@ -7,7 +7,8 @@ use std::collections::BTreeSet; /// One locate follow-up per function whose split raised a review or consider, /// per hardcoded-value function raised to a review or consider, per redundant -/// test pair raised to a review, and per test that asserts internal details. +/// test pair raised to a review, per test that asserts internal details, and +/// per injection consider that rests on its parameters. pub fn locates(plan: &Plan, files: &[FileResult]) -> Vec { follow_ups(plan, files, compose::unlocated_units, |unit| { match &unit.detail { @@ -15,7 +16,9 @@ pub fn locates(plan: &Plan, files: &[FileResult]) -> Vec { | Detail::Document { locate, .. } | Detail::Values { locate, .. } | Detail::Constants { locate, .. } => locate.as_ref(), - Detail::TestPair { confirm, .. } | Detail::Test { confirm } => confirm.as_ref(), + Detail::TestPair { confirm, .. } + | Detail::Test { confirm } + | Detail::Security { confirm, .. } => confirm.as_ref(), _ => None, } }) diff --git a/src/units/mod.rs b/src/units/mod.rs index a4e9ef3..f5baed3 100644 --- a/src/units/mod.rs +++ b/src/units/mod.rs @@ -186,6 +186,9 @@ pub enum Detail { /// trace and recheck, such as where its URLs come from or its output /// goes; each is asked only while its checks are undecided. settles: Vec, + /// For injection, what the values it places can hold, asked only + /// after a consider that rests on its parameters. + confirm: Option, /// Django code, asked the Django checks: a weak setting must be /// named by one of them. django: bool, diff --git a/src/units/outcome/injection.rs b/src/units/outcome/injection.rs index 62c7efc..0492a23 100644 --- a/src/units/outcome/injection.rs +++ b/src/units/outcome/injection.rs @@ -60,7 +60,19 @@ pub(in crate::units) fn injection_outcome<'a>( (_, Some(p)) => Outcome::Review(p), (outcome, None) => outcome, }; - Some(by_origin(origin, &found, get)) + Some(match by_origin(origin, &found, get) { + Outcome::Consider(p) if program_values(get) => Outcome::Note(p), + outcome => outcome, + }) +} + +/// Whether what a consider's values can hold, asked after it, leans toward +/// the program's own: literals its callers pass, values it creates, or the +/// arguments of a local tool. Such a consider, resting on the function's +/// parameters, is a note. +fn program_values<'a>(get: &impl Fn(&str) -> Option<&'a Answer>) -> bool { + choice_mass(get("values"), &questions::PROGRAM_VALUES) + .is_some_and(|p| probability_at_least(p, LEADING_PROBABILITY)) } /// When the markup check found a variable and the PHP markup Choice names diff --git a/src/units/questions/security.rs b/src/units/questions/security.rs index a2ec26d..e2a932e 100644 --- a/src/units/questions/security.rs +++ b/src/units/questions/security.rs @@ -185,6 +185,46 @@ pub fn security_origin(code: &str, callers: bool, django: bool) -> Value { ) } +/// What the values of an injection consider resting on the function's +/// parameters can hold, asked only for such a finding, with its callers. +/// Labeled by hand, those considers were right 30 times in 81: most wrong +/// ones placed text every caller passes as a literal, such as a Rust +/// helper's SQL fragments, or a command-line tool's own arguments, while +/// right ones placed names from a database others write, fetched page +/// titles or model output. Asked where the values come from, the recheck +/// answered "the function's parameters" at 0.9 even when its callers passed +/// literals. +pub fn injection_values(code: &str, callers: bool) -> Value { + let (fixed, note) = if callers { + ( + "Only text the program fixes: literals and constants, written in this code or passed by every caller in `callers`; numbers, dates or other typed values that cannot hold markup or syntax; or names chosen from a fixed list.", + format!("{CALLERS} {EVIDENCE}"), + ) + } else { + ( + "Only text the program fixes: literals and constants written in this code; numbers, dates or other typed values that cannot hold markup or syntax; or names chosen from a fixed list.", + EVIDENCE.to_string(), + ) + }; + json!({ + "type": "choice", + "instructions": { + "question": format!("What can the values that `{code}` places into a query, command, code or markup without binding or escaping them hold?"), + "note": note, + }, + "criteria": { + "fixed": fixed, + "own": "Values the program creates or keeps for itself, such as ids it generates, the names of its own tables, files or settings, or text it wrote itself.", + "local": "The arguments of a command-line program, build script or code generator, typed by the person who runs it on their own machine, or text that person runs on purpose, such as a query they typed.", + "outside": "Text another party can set: a network request, message or uploaded file, a page or feed fetched from the network, a language model's output, or records and names other users can write, such as rows of a shared database.", + "unknown": "Values from parameters or calls whose origin is not shown, which may hold any of these.", + }, + }) +} + +/// The options of `injection_values` that hold only the program's own values. +pub const PROGRAM_VALUES: [&str; 3] = ["fixed", "own", "local"]; + /// Asked in the sensitive-data trace: whether every error message is the /// program's own. It can only clear the error-detail signals; functions that /// throw the program's typed errors otherwise stayed undecided, since the diff --git a/src/units/security.rs b/src/units/security.rs index 90097ef..80c95b1 100644 --- a/src/units/security.rs +++ b/src/units/security.rs @@ -325,6 +325,9 @@ fn push_unit( let recheck = (rule == INJECTION) .then(|| recheck(file, subject, id)) .flatten(); + let confirm = (rule == INJECTION) + .then(|| confirm(file, subject, id)) + .flatten(); let settles = settles(file, subject, rule, id); out.units.push(UnitPlan { rule, @@ -344,6 +347,7 @@ fn push_unit( }, trace: trace.map(Into::into), settles, + confirm: confirm.map(Into::into), django: subject.django, test_path: subject.test_path, }, @@ -386,9 +390,16 @@ fn send( let unit = &mut out.units[*index]; unit.presence = Presence::NeedsContext; unit.recheck = None; - if let Detail::Security { trace, settles, .. } = &mut unit.detail { + if let Detail::Security { + trace, + settles, + confirm, + .. + } = &mut unit.detail + { *trace = None; settles.clear(); + *confirm = None; } } } @@ -718,15 +729,47 @@ fn recheck(file: &FileContext<'_>, subject: &Subject<'_>, id: &str) -> Option<(V Pass::Recheck, ); } + let (request, asked) = file.request("recheck", with_callers(file, subject), questions); + file.budget.fits(&request).then_some((request, asked)) +} + +/// The unit's code with the functions that call it and the enums its sites +/// name: the evidence of its recheck and of what its values can hold. +fn with_callers(file: &FileContext<'_>, subject: &Subject<'_>) -> Value { let mut state = json!({ "file": file.file_state(), subject.kind: subject.state(), - "callers": subject.callers.iter().map(|(name, source)| json!({"name": name, "source": source})).collect::>(), }); + if !subject.callers.is_empty() { + state["callers"] = json!( + subject + .callers + .iter() + .map(|(name, source)| json!({"name": name, "source": source})) + .collect::>() + ); + } if !subject.enums.is_empty() { state["enums_named_in_sites"] = json!(subject.enums); } - let (request, asked) = file.request("recheck", state, questions); + state +} + +/// What the values an injection consider rests on can hold, asked only +/// when its origin was the function's parameters: the function, and the +/// functions that call it. +fn confirm(file: &FileContext<'_>, subject: &Subject<'_>, id: &str) -> Option<(Value, Asked)> { + let code = subject.code(); + let mut questions = Questions::default(); + questions.ask( + "values".into(), + questions::injection_values(&code, !subject.callers.is_empty()), + id, + INJECTION, + "values", + Pass::Locate, + ); + let (request, asked) = file.request("locate", with_callers(file, subject), questions); file.budget.fits(&request).then_some((request, asked)) } diff --git a/src/units/tests/security.rs b/src/units/tests/security.rs index fa921ba..791b03b 100644 --- a/src/units/tests/security.rs +++ b/src/units/tests/security.rs @@ -70,6 +70,9 @@ fn an_unhandled_value_from_another_party_is_a_located_injection_review() { assert!(finding.action.contains("bound query parameters")); } +/// The options of the Choice on what an injection consider's values can hold. +const VALUES: [&str; 5] = ["fixed", "local", "outside", "own", "unknown"]; + #[test] fn a_parameter_origin_is_a_consider_that_callers_can_settle() { let caller = format!( @@ -80,6 +83,7 @@ fn a_parameter_origin_is_a_consider_that_callers_can_settle() { ("interpreted", noul_at(0.95)), ("sql", noul_at(0.95)), ("origin", spread(0.0, 0.9, 0.1)), + ("values", choice_of("unknown", &VALUES)), ] }; let (project, options) = security_project(QUERY); @@ -96,7 +100,7 @@ fn a_parameter_origin_is_a_consider_that_callers_can_settle() { eval.overrides = overrides(); eval.recheck_level = Some(0); let report = run(&project, &options, &mut eval); - assert_eq!(eval.stages.last().unwrap(), "recheck"); + assert!(eval.stages.contains(&"recheck".to_string())); let find = |report: &Report| { report.files[0] .findings @@ -106,6 +110,56 @@ fn a_parameter_origin_is_a_consider_that_callers_can_settle() { assert!(!find(&report), "the caller passes a fixed value"); } +#[test] +fn a_parameter_consider_is_a_note_when_its_values_are_the_programs_own() { + let caller = format!( + "{QUERY}\nfn handler(conn: &Connection) -> Result {{\n find(conn, \"admin\")\n}}\n" + ); + let (project, mut options) = security_project(&caller); + let mut judged = |values: Value| { + let mut eval = scripted(0); + eval.overrides = vec![ + ("interpreted", noul_at(0.95)), + ("sql", noul_at(0.95)), + ("origin", spread(0.0, 0.9, 0.1)), + ]; + // The recheck with callers keeps the parameters as the origin. + eval.recheck_overrides = vec![ + ("sql", noul_at(0.95)), + ("origin", spread(0.0, 0.9, 0.1)), + ("values", values), + ]; + let report = run(&project, &options, &mut eval); + options.refresh = true; + report.files[0] + .findings + .iter() + .find(|f| f.rule == "security/injection" && f.symbol.as_deref() == Some("find")) + .map(|f| f.strength) + }; + assert_eq!( + judged(choice_of("fixed", &VALUES)), + Some(Strength::Note), + "the caller passes a literal" + ); + assert_eq!( + judged(choice_of("outside", &VALUES)), + Some(Strength::Consider) + ); + let mut split: serde_json::Map = + VALUES.iter().map(|k| (k.to_string(), json!(0.0))).collect(); + split.insert("own".into(), json!(0.3)); + split.insert("local".into(), json!(0.25)); + split.insert("unknown".into(), json!(0.45)); + let leaning = + json!({"type":"choice","choice":"unknown","confidence":0.4,"probabilities":split}); + assert_eq!( + judged(leaning), + Some(Strength::Note), + "the program's own options together lead" + ); +} + #[test] fn a_check_left_undecided_is_decided_again_with_callers() { let caller = format!( @@ -1277,6 +1331,8 @@ fn settled_status( let mut eval = scripted(0); eval.overrides = nouls.iter().map(|&(q, p)| (q, noul_at(p))).collect(); eval.overrides.push(("origin", spread(0.0, 0.9, 0.1))); + eval.overrides + .push(("values", choice_of("unknown", &VALUES))); eval.overrides.push(settle); let report = run(project, options, &mut eval); ( From 560f3302b275cb7f658ca0db940466f5f6f1dd51 Mon Sep 17 00:00:00 2001 From: Tauan BF <11513929+tauanbinato@users.noreply.github.com> Date: Sat, 26 Sep 2026 18:14:30 -0300 Subject: [PATCH 2/9] Show where else a hardcoded value is written, and ask a consider's value what it is The Choice that names a finding's value lists, with each value, the other lines of its file that write it. A consider that rests only on a value's name, whose file writes that value again, is then asked what the value is: copies that must change together, or a value nothing explains, keep it; a value its field, argument or comment explains, an idiom or a hand-tuned number, clearly chosen, make it a note. Labeled such considers went from 53 right and 77 wrong to 32 right and 28 wrong. --- CHANGELOG.md | 1 + docs/classification-cascade.md | 9 +- site/src/how-it-works.md | 9 +- src/catalog.rs | 2 +- src/evaluate.rs | 6 +- src/units/compose.rs | 66 +++++++++++- src/units/follow_ups.rs | 21 ++++ src/units/hardcoded.rs | 134 ++++++++++++++++++++----- src/units/mod.rs | 2 +- src/units/outcome/mod.rs | 2 +- src/units/outcome/security.rs | 2 +- src/units/questions/maintainability.rs | 33 +++++- src/units/tests/hardcoded.rs | 72 +++++++++++++ 13 files changed, 322 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91201a3..2c1c05a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ Measured on 103 pinned projects (24 new open-source ones of kinds not tried befo - Project templates: a file under a directory whose name holds a `{{ … }}` placeholder, as a cookiecutter template's `{{cookiecutter.project_slug}}` does, is parsed without its Jinja tags (statements and comments blanked, each placeholder read as a name of the same length, so lines stay the file's) and judged; the evidence keeps the tags. 31 of cookiecutter-django's Python and JavaScript files, the generated application's settings, models, views and tests, were skipped for syntax errors and are judged; its Celery settings turning off Redis certificate checks (`ssl.CERT_NONE`) are a review. - Tests: a test said to assert internal details is asked, with the bodies of the functions it calls, what its assertions read: results, state the program shows or acts on next, or effects a caller observes clear the consider; stored input or calls between the program's own functions keep it. Asked of the test and the signatures it calls, the check read a debug panel's recorded queries (`panel._queries`, which the panel renders), Devise's documented hooks and an app's state after an action as internals: 49 of 66 such considers labeled on the corpus were wrong. A test that reads members through reflection or a cast to `any` keeps its consider without being asked. Labeled tests/value considers went from 29% to 77% right (20 right and 6 wrong, against 22 and 55), and on held-out projects from 2 right and 13 wrong to 1 wrong. About $0.006 of follow-ups on the corpus. - Injection: a consider that rests on the function's parameters ("a caller passing outside input would make it exploitable") is asked, with the functions that call it, what the values it places can hold. It becomes a note when text the program fixes (literals its callers pass, numbers, names from a fixed list), values it creates or a local tool's own arguments lead; text another party can set, or callers that are not shown, keep it. Asked where the values come from, the recheck answered "the function's parameters" at 0.9 even for a Rust helper whose four callers pass literal SQL fragments. On the labeled projects, such considers went from 30 right and 51 wrong to 30 right and 32 wrong. Only these follow-ups are asked. +- Hardcoded values: the Choice that names a finding's value lists, with each value, the other lines of its file that write it. A consider that rests only on a value's name, whose file writes that value again, is then asked what the value is: copies that must change together, or a value nothing near it explains, keep it; a value that the field or argument it fills or a comment beside it explains, an idiom such as a tolerance near zero or a unit conversion, or a hand-tuned number, clearly chosen, make it a note. On the labeled projects, such considers went from 53 right and 77 wrong (41%) to 32 right and 28 wrong (53%); the others are notes. Offered instead as the locate's "no value stands out", those kinds also took values with copies that must stay equal. Only these follow-ups are asked. - Tests: Deno tests are test cases, in each of their forms: `Deno.test("name", fn)`, `Deno.test({ name: "name", fn() {…} })` and `Deno.test(function name() {…})`, with `.only` and `.ignore`. oak writes its 266 tests in the object form, and none of them was judged: its test files got a file-purpose request each and the test rules found nothing to ask. Only Deno projects' requests change. ## [0.20.0] - 2026-09-26 diff --git a/docs/classification-cascade.md b/docs/classification-cascade.md index ce2447d..1d71e6a 100644 --- a/docs/classification-cascade.md +++ b/docs/classification-cascade.md @@ -190,7 +190,14 @@ signatures, or one candidate pair. again with the body of the function both call: whether it throws before the rest of a test runs is in that body. Then one locate Choice per split finding picks the body block to extract, - and one per hardcoded-value review or consider names the value it is about. + and one per hardcoded-value review or consider names the value it is about, + each value listed with the other lines of its file that write it. A + consider that rests only on a value's name, whose file writes that value + again, is then asked what the value is: copies that must change together, + or a value nothing near it explains, keep it; a value that the field or + argument it fills or a comment beside it explains, an idiom such as a + tolerance near zero or a unit conversion, or a hand-tuned number make it a + note. Special-case findings in different files that name the same identity become one finding at the strongest site; the others are notes pointing at it. Numbers and paths are not grouped: `1000` meant metres per kilometre in diff --git a/site/src/how-it-works.md b/site/src/how-it-works.md index 2663c47..0c1302c 100644 --- a/site/src/how-it-works.md +++ b/site/src/how-it-works.md @@ -208,7 +208,14 @@ signatures, or one candidate pair. again with the body of the function both call: whether it throws before the rest of a test runs is in that body. Then one locate Choice per split finding picks the body block to extract, - and one per hardcoded-value review or consider names the value it is about. + and one per hardcoded-value review or consider names the value it is about, + each value listed with the other lines of its file that write it. A + consider that rests only on a value's name, whose file writes that value + again, is then asked what the value is: copies that must change together, + or a value nothing near it explains, keep it; a value that the field or + argument it fills or a comment beside it explains, an idiom such as a + tolerance near zero or a unit conversion, or a hand-tuned number make it a + note. Special-case findings in different files that name the same identity become one finding at the strongest site; the others are notes pointing at it. Numbers and paths are not grouped: `1000` meant metres per kilometre in diff --git a/src/catalog.rs b/src/catalog.rs index a1349c0..55d2adb 100644 --- a/src/catalog.rs +++ b/src/catalog.rs @@ -279,7 +279,7 @@ pub fn rule_version(key: &str) -> &'static str { TEST_REDUNDANCY => "4", INJECTION => "9", SENSITIVE_DATA => "7", - HARDCODED_VALUES | UNSAFE_SETTINGS => "5", + HARDCODED_VALUES | UNSAFE_SETTINGS => "6", AGENT_CONTEXT => "3", COMMENTS => "3", LARGE_DOCS => "3", diff --git a/src/evaluate.rs b/src/evaluate.rs index d90e1d4..c88c426 100644 --- a/src/evaluate.rs +++ b/src/evaluate.rs @@ -216,8 +216,9 @@ impl Session<'_> { // Traces judge where a security concern's values come from; rechecks // settle uncertain units; a security check still undecided is asked // where its URL comes from or its output goes, and an outline its kind; - // locate follow-ups then point split findings at a block. Each depends - // on the answers before it. + // locate follow-ups then point split findings at a block, and a + // located value is asked what it is. Each depends on the answers + // before it. for follow_up in [ crate::units::doc_checks, crate::units::traces, @@ -225,6 +226,7 @@ impl Session<'_> { crate::units::settles, crate::units::kinds, crate::units::locates, + crate::units::value_kinds, ] { let tasks: Vec<_> = follow_up(&plan, &report.files) .iter() diff --git a/src/units/compose.rs b/src/units/compose.rs index 5c2f747..0c2209e 100644 --- a/src/units/compose.rs +++ b/src/units/compose.rs @@ -3,8 +3,9 @@ use super::{ Access, Block, Detail, FilePlan, Presence, UnitPlan, outcome::{ - Answers, Outcome, at_most_note, benefit, checks, choice, document_split, lowered, noul, - open, origin_outcome, score, settled_checks, several_kind, unit_outcome, value_signals, + Answers, Outcome, at_most_note, benefit, checks, choice, choice_mass, document_split, + lowered, noul, open, origin_outcome, score, settled_checks, several_kind, unit_outcome, + value_signals, }, wording::{Wording, comment_reason, comment_wording}, wording::{ @@ -1088,7 +1089,11 @@ fn capped( if unnamed_value(unit, judgments) { return lowered(lowered(outcome)); } - if single_use_value(unit, judgments) || short_outline(unit) || small_section(unit) { + if single_use_value(unit, judgments) + || readable_value(unit, judgments) + || short_outline(unit) + || small_section(unit) + { return at_most_note(outcome); } if named_value_only(unit, judgments) { @@ -1101,6 +1106,59 @@ fn capped( if lower { lowered(outcome) } else { outcome } } +/// The follow-up of each hardcoded-value consider that rests on a value's +/// name, whose value the locate named and its file writes again, not yet +/// asked what that value is. +pub fn unkinded_values( + plan: &FilePlan, + judgments: &[Judgment], +) -> Vec<(serde_json::Value, super::Asked)> { + plan.units + .iter() + .filter(|u| u.presence == Presence::Judged) + .filter_map(|u| { + let Detail::Values { + locate: Some(locate), + repeated, + .. + } = &u.detail + else { + return None; + }; + if answers(judgments, &u.id, Pass::Locate).contains_key("value_kind") + || !named_value_only(u, judgments) + { + return None; + } + let option = located_option(u, judgments, ("value", 'v'))?; + if repeated.get(option) != Some(&true) { + return None; + } + super::hardcoded::value_kind(locate, option, &u.id) + }) + .collect() +} + +/// Such a consider whose value, asked what it is, clearly reads for itself +/// where it is used: the field or argument it fills or a comment beside it +/// says what it is, or it is an idiom or a hand-tuned number, together at +/// the threshold of a clear located part. Its finding is a note; a value +/// with copies that must change together, or that nothing explains, stays +/// a consider. Leaning toward those kinds was not enough: at 0.50 they took +/// 30 of 46 right considers with 47 of 64 wrong ones. +fn readable_value(unit: &UnitPlan, judgments: &[Judgment]) -> bool { + named_value_only(unit, judgments) + && choice_mass( + answers(judgments, &unit.id, Pass::Locate) + .get("value_kind") + .copied(), + &super::questions::READABLE_VALUES, + ) + .is_some_and(|p| { + crate::policy::probability_at_least(p, crate::policy::LOCATION_PROBABILITY) + }) +} + /// A function's hardcoded-value review or consider whose value was not /// named: the locate Choice picked none clearly, or there were too many /// values to offer. Its finding is a note, since a reader cannot tell what @@ -1231,6 +1289,8 @@ fn lowered_value(unit: &UnitPlan, judgments: &[Judgment]) -> Option<(Strength, & "No single value stood out, so it is a note." } else if single_use_value(unit, judgments) { "It is written once in its file, so it is a note." + } else if readable_value(unit, judgments) { + "It reads for itself where it is used, so it is a note." } else if named_value_only(unit, judgments) && matches!(resolved(unit, judgments).0, Outcome::Review(_)) { diff --git a/src/units/follow_ups.rs b/src/units/follow_ups.rs index 804d922..6b4376f 100644 --- a/src/units/follow_ups.rs +++ b/src/units/follow_ups.rs @@ -24,6 +24,27 @@ pub fn locates(plan: &Plan, files: &[FileResult]) -> Vec { }) } +/// One question per hardcoded-value consider resting on a value's name +/// whose value its file writes again: what that value is. It needs the +/// value the locate named, so it follows the locates. +pub fn value_kinds(plan: &Plan, files: &[FileResult]) -> Vec { + let mut planned = Vec::new(); + for (&owner, file_plan) in &plan.files { + let file = &files[owner]; + if file.status == Status::Error { + continue; + } + for (request, asked) in compose::unkinded_values(file_plan, &file.judgments) { + planned.push(Planned { + owner, + request, + asked, + }); + } + } + planned +} + /// The section and pair checks of documents that are not finished plans. pub fn doc_checks(plan: &Plan, files: &[FileResult]) -> Vec { let finished = compose::finished_plans(plan, files); diff --git a/src/units/hardcoded.rs b/src/units/hardcoded.rs index b204c9e..9bf71c2 100644 --- a/src/units/hardcoded.rs +++ b/src/units/hardcoded.rs @@ -4,15 +4,16 @@ //! a Noul on whether the function special-cases one identity. A unit left //! undecided is asked, alone, whether every value is of an acceptable kind. use super::{ - Detail, FileContext, FilePlan, Planned, Presence, Questions, UnitPlan, compact, identity, - pack_runs, questions, unique_ids, + Asked, Detail, FileContext, FilePlan, FollowUp, Planned, Presence, Questions, UnitPlan, + compact, identity, pack_runs, questions, unique_ids, }; use crate::{ - analysis::{literals::Constant, units::Unit}, + analysis::{literals::Constant, sites::clip, units::Unit}, catalog::HARDCODED_VALUES, schema::Pass, }; use serde_json::{Value, json}; +use std::path::Path; pub(super) fn plan( file: &FileContext<'_>, @@ -49,7 +50,7 @@ pub(super) fn plan( } } let locate = (choices.len() <= LOCATE_CHOICES) - .then(|| locate(file, &unit.name, source, &id, &choices)); + .then(|| locate(file, (unit, source), &id, &choices)); Detail::Values { values: unit.literals.iter().map(|l| l.text.clone()).collect(), repeated: choices @@ -82,46 +83,132 @@ pub(super) fn plan( /// Most distinct values a locate Choice offers; a unit with more is not located. const LOCATE_CHOICES: usize = 24; -/// How often a literal is written in `source`: a number as a whole token (not +/// How often a literal is written in `source`. +fn occurrences(source: &str, literal: &str) -> usize { + written_at(source, literal).count() +} + +/// Where a literal is written in `source`: a number as a whole token (not /// part of `100` or `10.5` for `10`), other text wherever it appears without /// its quotes. -fn occurrences(source: &str, literal: &str) -> usize { +fn written_at<'a>(source: &'a str, literal: &'a str) -> impl Iterator + 'a { let text = literal.trim_matches(['"', '\'', '`']); - if text.is_empty() { - return 0; - } let number = text.starts_with(|c: char| c.is_ascii_digit() || c == '-' || c == '.'); let word = |c: char| c.is_alphanumeric() || c == '_' || c == '.'; - source - .match_indices(text) - .filter(|(at, _)| { + (!text.is_empty()) + .then_some(text) + .into_iter() + .flat_map(move |text| source.match_indices(text)) + .filter(move |(at, _)| { !number || !(source[..*at].chars().next_back().is_some_and(word) || source[at + text.len()..].chars().next().is_some_and(word)) }) - .count() + .map(|(at, _)| at) +} + +/// Lines of the file outside a function shown with each of its values, at most. +const ELSEWHERE_LINES: usize = 3; + +/// The lines of `source` outside `lines` that write `literal`, numbered. +fn elsewhere(source: &str, literal: &str, lines: (usize, usize)) -> Vec { + let mut found: Vec<(usize, String)> = Vec::new(); + let (mut line, mut counted) = (1, 0); + for at in written_at(source, literal) { + line += source.as_bytes()[counted..at] + .iter() + .filter(|b| **b == b'\n') + .count(); + counted = at; + if (lines.0..=lines.1).contains(&line) || found.last().is_some_and(|(l, _)| *l == line) { + continue; + } + let start = source[..at].rfind('\n').map_or(0, |i| i + 1); + let end = source[at..].find('\n').map_or(source.len(), |i| at + i); + found.push((line, format!("{line}: {}", clip(source[start..end].trim())))); + if found.len() == ELSEWHERE_LINES { + break; + } + } + found.into_iter().map(|(_, text)| text).collect() } -/// Which value a finding is about: the function's source and its distinct values. +/// Which value a finding is about: the function's source and its distinct +/// values, each with the other lines of the file that write it. Without +/// them, a value that must stay equal to a copy in another function read as +/// clear where it was used, and a value that merely recurs, such as the 4 +/// of quarters in a year, read as a value to share. fn locate( file: &FileContext<'_>, - name: &str, - source: &str, + (unit, source): (&Unit, &str), id: &str, choices: &[String], -) -> (Value, super::Asked) { +) -> (Value, Asked) { let ids = option_ids('v', choices.len()); + let values: Vec = ids + .iter() + .zip(choices) + .map(|(id, value)| { + let mut entry = json!({"id": id, "value": value}); + let lines = elsewhere(file.source, value, (unit.line, unit.end_line)); + if !lines.is_empty() { + entry["elsewhere"] = json!(lines); + } + entry + }) + .collect(); let state = json!({ "file": file.file_state(), "function": { - "name": name, + "name": unit.name, "source": source, - "values": ids.iter().zip(choices).map(|(id, value)| json!({"id": id, "value": value})).collect::>(), + "values": values, }, }); locate_request(file, id, ("value", questions::hardcoded_value(&ids)), state) } +/// What a consider's value is, asked about the value its locate named: the +/// function, the value and the other lines of its file that write it, taken +/// from the locate request. +pub(super) fn value_kind(locate: &FollowUp, option: usize, id: &str) -> Option<(Value, Asked)> { + let located = locate.request(); + let function = &located["state"]["function"]; + let entry = function["values"].get(option)?; + let mut state = json!({ + "file": located["state"]["file"], + "function": {"name": function["name"], "source": function["source"]}, + "value": entry["value"], + }); + if let Some(lines) = entry.get("elsewhere") { + state["elsewhere"] = lines.clone(); + } + let mut questions = Questions::default(); + questions.ask( + "value_kind".into(), + questions::hardcoded_value_kind(), + id, + HARDCODED_VALUES, + "value_kind", + Pass::Locate, + ); + let language = located["state"]["file"]["language"] + .as_str() + .unwrap_or_default(); + let sources: Vec<(&Path, &str)> = located["jevgate"]["sources"] + .as_array()? + .iter() + .filter_map(|s| Some((Path::new(s["path"].as_str()?), s["source_hash"].as_str()?))) + .collect(); + Some(super::evidence::request( + located["model"].as_str()?, + "locate", + &sources, + state, + questions.reworded(language), + )) +} + /// Option ids `{prefix}0`, `{prefix}1`, … for a locate Choice over `count` entries. fn option_ids(prefix: char, count: usize) -> Vec { (0..count).map(|i| format!("{prefix}{i}")).collect() @@ -133,7 +220,7 @@ fn locate_request( id: &str, (question, body): (&'static str, Value), state: Value, -) -> (Value, super::Asked) { +) -> (Value, Asked) { let mut questions = Questions::default(); questions.ask( question.into(), @@ -177,10 +264,7 @@ fn send_or_split( } } -fn functions_request( - file: &FileContext<'_>, - items: &[(usize, String, Value)], -) -> (Value, super::Asked) { +fn functions_request(file: &FileContext<'_>, items: &[(usize, String, Value)]) -> (Value, Asked) { let mut questions = Questions::default(); for (index, (_, id, _)) in items.iter().enumerate() { let values = format!("functions[{index}].values"); @@ -302,7 +386,7 @@ fn benign_request( id: &str, evidence: Value, function: bool, -) -> Option<(Value, super::Asked)> { +) -> Option<(Value, Asked)> { let (values, code, asked): (&str, &str, &[&'static str]) = if function { ( "functions[0].values", diff --git a/src/units/mod.rs b/src/units/mod.rs index f5baed3..355349e 100644 --- a/src/units/mod.rs +++ b/src/units/mod.rs @@ -32,7 +32,7 @@ mod workflows; use answers::Questions; pub use answers::{Asked, record}; use evidence::{FileContext, compact, identity, pack, pack_runs, request, unique_ids}; -pub use follow_ups::{doc_checks, kinds, locates, rechecks, settles, traces}; +pub use follow_ups::{doc_checks, kinds, locates, rechecks, settles, traces, value_kinds}; use plan::Scope; pub use plan::plan; pub use spacetimedb::spacetimedb_module; diff --git a/src/units/outcome/mod.rs b/src/units/outcome/mod.rs index 9b409b4..a0dd07a 100644 --- a/src/units/outcome/mod.rs +++ b/src/units/outcome/mod.rs @@ -34,7 +34,7 @@ pub(super) use maintainability::{ }; use pairs::doc_pair_outcome; pub(super) use pairs::{disagreement, pair_signals, repeated}; -pub(super) use security::{checks, settled_checks}; +pub(super) use security::{checks, choice_mass, settled_checks}; pub(super) use test_rules::{redundancy_outcome, test_value_outcome}; #[derive(Clone, Copy, Debug, PartialEq)] diff --git a/src/units/outcome/security.rs b/src/units/outcome/security.rs index c203586..0914cdf 100644 --- a/src/units/outcome/security.rs +++ b/src/units/outcome/security.rs @@ -55,7 +55,7 @@ pub(super) fn settled<'a>( } /// The share of a Choice's probability on `options`, when it was answered. -pub(super) fn choice_mass(answer: Option<&Answer>, options: &[&str]) -> Option { +pub(in crate::units) fn choice_mass(answer: Option<&Answer>, options: &[&str]) -> Option { let Answer::Choice { probabilities, .. } = answer? else { return None; }; diff --git a/src/units/questions/maintainability.rs b/src/units/questions/maintainability.rs index 14b4a95..5cc5294 100644 --- a/src/units/questions/maintainability.rs +++ b/src/units/questions/maintainability.rs @@ -191,12 +191,43 @@ pub fn hardcoded_constant(ids: &[String]) -> Value { pub fn hardcoded_value(ids: &[String]) -> Value { choose_id( "Which value in `function.values` most needs to come from configuration, get a descriptive name, or be read from data instead of being written in `function.source`?", - format!("Options are the `id` values in `function.values`. {EVIDENCE}"), + format!( + "Options are the `id` values in `function.values`; a value's `elsewhere` lists other lines of the file that write it. {EVIDENCE}" + ), ids, "No single value stands out.", ) } +/// Asked only about the value a consider resting on a value's name names, +/// when its file writes that value again: what a reader needs to understand +/// it. Labeled by hand, 77 of 130 such considers were wrong, their values +/// explained by the field or argument they fill, a comment beside them, or +/// an idiom such as a zero-length guard, while right ones had copies that +/// must change together. Offered as the locate's `none`, those kinds also +/// took values with such copies: 6 right considers became notes for 17 +/// wrong ones. Told that copies win over the other kinds, it chose them for +/// wrong considers too: 22 became notes instead of 42. +pub fn hardcoded_value_kind() -> Value { + json!({ + "type": "choice", + "instructions": { + "question": "What best describes `value` where `function.source` uses it?", + "note": format!("`elsewhere` lists other lines of the file that write the same value. {EVIDENCE}"), + }, + "criteria": { + "copies": "It stands for the same quantity as a copy in `elsewhere` or in this function, and the copies must change together while nothing ties them.", + "unexplained": "Nothing near it says what it stands for or why it has this value.", + "named": "The parameter, field, variable or function it goes into, or a comment beside it, says what it is.", + "idiom": "A common constant or idiom that reads for itself, such as a tolerance near zero, a half, a unit conversion such as 60, 1000 or 100 for percent, or a size a format fixes.", + "tuning": "One of many hand-tuned numbers for look, sound, motion or layout, whose exact value is a matter of taste.", + }, + }) +} + +/// The options of `hardcoded_value_kind` under which a value reads for itself. +pub const READABLE_VALUES: [&str; 3] = ["named", "idiom", "tuning"]; + /// Whether a value fixed in code changes between environments. `values` names /// the list of candidate values; `code` describes the code that uses them. /// Criteria name what is not environment-specific (the program's own routes, diff --git a/src/units/tests/hardcoded.rs b/src/units/tests/hardcoded.rs index 9bf07ed..cfb0a32 100644 --- a/src/units/tests/hardcoded.rs +++ b/src/units/tests/hardcoded.rs @@ -162,6 +162,78 @@ fn a_value_that_needs_a_name_but_is_written_once_is_a_note() { ); } +#[test] +fn a_named_value_that_reads_for_itself_is_a_note() { + let source = format!( + "{HARDCODED}\nfn backup() -> Client {{\n Client::new(\"db.backup:5432\", 30_000)\n}}\n" + ); + let (project, mut options) = rule_project(&source, catalog::HARDCODED_VALUES); + let kinds = ["copies", "idiom", "named", "tuning", "unexplained"]; + let mut connect = |kind: &str| { + let mut eval = scripted(0); + eval.overrides = vec![ + ("magic", spread(0.1, 0.35, 0.55)), + ("value", choice_of("v1", &["v0", "v1", "none"])), + ("value_kind", choice_of(kind, &kinds)), + ]; + let report = run(&project, &options, &mut eval); + options.refresh = true; + let asked = report + .stages + .get("locate") + .map_or(0, |stage| stage.successful_requests); + let finding = report.files[0] + .findings + .iter() + .find(|f| f.symbol.as_deref() == Some("connect")) + .cloned() + .unwrap(); + (finding, asked) + }; + // Copies that must change together keep the consider. + let (kept, asked) = connect("copies"); + assert_eq!(kept.strength, Strength::Consider, "{}", kept.message); + // Both functions' values are located, then both are asked their kind. + assert_eq!(asked, 4); + let (named, _) = connect("named"); + assert_eq!(named.strength, Strength::Note); + assert!( + named + .message + .ends_with("It reads for itself where it is used, so it is a note."), + "{}", + named.message + ); +} + +#[test] +fn the_value_locate_lists_the_other_lines_that_write_each_value() { + let source = format!( + "{HARDCODED}\nfn backup() -> Client {{\n Client::new(\"db.backup:5432\", 30_000)\n}}\n\nconst CAP: u64 = 300_000;\n" + ); + let (project, options) = rule_project(&source, catalog::HARDCODED_VALUES); + let (_, plan) = planned(&project, &options); + let locate = plan.files[&0] + .units + .iter() + .find_map(|u| match &u.detail { + Detail::Values { + locate: Some(locate), + .. + } if u.name == "connect" => Some(locate.request()), + _ => None, + }) + .expect("a locate for connect"); + let values = &locate["state"]["function"]["values"]; + assert_eq!(values[0]["value"], "\"db.internal:5432\""); + assert!(values[0].get("elsewhere").is_none(), "written once"); + // `300_000` holds `30_000` only as part of a longer number. + assert_eq!( + values[1]["elsewhere"], + json!(["12: Client::new(\"db.backup:5432\", 30_000)"]) + ); +} + #[test] fn undecided_units_are_listed_with_the_questions_left_undecided() { let (project, mut options) = function_rule_project(&function("borderline")); From f6e4e67a4abc38c35c2b1586ee6849aa3b8d5fe2 Mon Sep 17 00:00:00 2001 From: Tauan BF <11513929+tauanbinato@users.noreply.github.com> Date: Sat, 26 Sep 2026 18:14:30 -0300 Subject: [PATCH 3/9] Ask whether code accepts what a server presents unverified, and keep i18next's escaping in React Asked whether code turns certificate verification off, Online Boutique's gRPC clients with insecure credentials, telemetry exporters and a database behind the AlloyDB connector were seven reviews whatever the check's examples said. The TLS check now asks whether code accepts a server's certificate, host key or host name without verifying it: a connection without TLS presents nothing, and verification skipped only when a caller or the operator asks is not turned off. The escape check names i18next's escapeValue: false in a React app as escaping kept. Labeled unsafe-settings reviews went from 54 right and 24 wrong to 54 and 14. --- CHANGELOG.md | 1 + docs/classification-cascade.md | 11 ++++++++++- site/src/how-it-works.md | 10 +++++++++- src/units/questions/security.rs | 26 +++++++++++++++++++++----- 4 files changed, 41 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c1c05a..c352b43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ Measured on 103 pinned projects (24 new open-source ones of kinds not tried befo - Tests: a test said to assert internal details is asked, with the bodies of the functions it calls, what its assertions read: results, state the program shows or acts on next, or effects a caller observes clear the consider; stored input or calls between the program's own functions keep it. Asked of the test and the signatures it calls, the check read a debug panel's recorded queries (`panel._queries`, which the panel renders), Devise's documented hooks and an app's state after an action as internals: 49 of 66 such considers labeled on the corpus were wrong. A test that reads members through reflection or a cast to `any` keeps its consider without being asked. Labeled tests/value considers went from 29% to 77% right (20 right and 6 wrong, against 22 and 55), and on held-out projects from 2 right and 13 wrong to 1 wrong. About $0.006 of follow-ups on the corpus. - Injection: a consider that rests on the function's parameters ("a caller passing outside input would make it exploitable") is asked, with the functions that call it, what the values it places can hold. It becomes a note when text the program fixes (literals its callers pass, numbers, names from a fixed list), values it creates or a local tool's own arguments lead; text another party can set, or callers that are not shown, keep it. Asked where the values come from, the recheck answered "the function's parameters" at 0.9 even for a Rust helper whose four callers pass literal SQL fragments. On the labeled projects, such considers went from 30 right and 51 wrong to 30 right and 32 wrong. Only these follow-ups are asked. - Hardcoded values: the Choice that names a finding's value lists, with each value, the other lines of its file that write it. A consider that rests only on a value's name, whose file writes that value again, is then asked what the value is: copies that must change together, or a value nothing near it explains, keep it; a value that the field or argument it fills or a comment beside it explains, an idiom such as a tolerance near zero or a unit conversion, or a hand-tuned number, clearly chosen, make it a note. On the labeled projects, such considers went from 53 right and 77 wrong (41%) to 32 right and 28 wrong (53%); the others are notes. Offered instead as the locate's "no value stands out", those kinds also took values with copies that must stay equal. Only these follow-ups are asked. +- Unsafe settings: outside C#, PHP and Django, the TLS check asks whether code accepts a server's certificate, host key or host name without verifying it, and names a connection made without TLS (gRPC's insecure credentials, an insecure OpenTelemetry exporter, `sslmode=disable`) and verification skipped only when a caller or the operator asks for it as not turning it off. Asked whether code turns verification off, Online Boutique's gRPC clients and telemetry exporters inside its cluster, and its database reached through the AlloyDB connector, were seven reviews whatever the examples said, and httpx's `verify=False` branch and vaultwarden's opt-in setting for invalid SMTP certificates were reviews too. The escape check names i18next's `escapeValue: false` in a React app, whose components escape what they render, as escaping kept. Labeled unsafe-settings reviews went from 54 right and 24 wrong to 54 right and 14 wrong; pgweb defaulting a bookmark's `sslmode` to `disable`, which turns off TLS rather than verification, is no longer one. Only unsafe-settings traces are asked again. - Tests: Deno tests are test cases, in each of their forms: `Deno.test("name", fn)`, `Deno.test({ name: "name", fn() {…} })` and `Deno.test(function name() {…})`, with `.only` and `.ignore`. oak writes its 266 tests in the object form, and none of them was judged: its test files got a file-purpose request each and the test rules found nothing to ask. Only Deno projects' requests change. ## [0.20.0] - 2026-09-26 diff --git a/docs/classification-cascade.md b/docs/classification-cascade.md index 1d71e6a..c3c9239 100644 --- a/docs/classification-cascade.md +++ b/docs/classification-cascade.md @@ -258,7 +258,16 @@ signatures, or one candidate pair. trace in C# also gets the `const` and `static readonly` fields the code names, often declared in another file, so a key written in the code does not read as configuration. Other languages keep their wording: the - additions were measured on ASP.NET Core projects only. A broad weak-setting + additions were measured on ASP.NET Core projects only. Outside C#, PHP and + Django, the TLS check asks whether code accepts a server's certificate, + host key or host name without verifying it, so a connection made without + TLS (gRPC's insecure credentials, an insecure OpenTelemetry exporter, + `sslmode=disable`) presents nothing to verify, and verification skipped + only when a caller or the operator asks for it is not turned off: asked + whether code turns verification off, a microservices demo's + cluster-internal gRPC clients stayed reviews whatever the examples said. + The escape check names i18next's `escapeValue: false` in a React app as + escaping kept. A broad weak-setting answer that none of the specific checks leans toward names no setting to change and is at most a note: on an action marked `[AllowAnonymous]` on purpose it was 0.85 while every check stayed at 0.30 or less. diff --git a/site/src/how-it-works.md b/site/src/how-it-works.md index 0c1302c..526421b 100644 --- a/site/src/how-it-works.md +++ b/site/src/how-it-works.md @@ -284,7 +284,15 @@ signatures, or one candidate pair. `escape_html_entities_in_json = false`) and whether passwords are saved or checked as plain text: in DVGA, JavaVulnerableLab, NodeGoat and RailsGoat the broad question found these at 0.93 to 0.98 while no check - named the setting, so they were notes. A broad weak-setting + named the setting, so they were notes. The TLS check asks whether code + accepts a server's certificate, host key or host name without verifying + it, so a connection made without TLS (gRPC's insecure credentials, an + insecure OpenTelemetry exporter, `sslmode=disable`) presents nothing to + verify, and verification skipped only when a caller or the operator asks + for it is not turned off: asked whether code turns verification off, a + microservices demo's cluster-internal gRPC clients stayed reviews + whatever the examples said. The escape check names i18next's + `escapeValue: false` in a React app as escaping kept. A broad weak-setting answer that none of the specific checks leans toward names no setting to change and is at most a note: on an action marked `[AllowAnonymous]` on purpose it was 0.85 while every check stayed at 0.30 or less. diff --git a/src/units/questions/security.rs b/src/units/questions/security.rs index e2a932e..7dee56a 100644 --- a/src/units/questions/security.rs +++ b/src/units/questions/security.rs @@ -478,13 +478,27 @@ pub const VIEW_MARKUP: Check = Check { /// Specific weak settings, asked when the broad presence question is not clear. /// Turning off output escaping had no check: NodeGoat's `autoescape: false` /// and RailsGoat's `escape_html_entities_in_json = false` were at most notes. +/// The TLS check's examples name what turns no verification off: in +/// microservices-demo, gRPC's insecure credentials between the cluster's +/// services, OpenTelemetry exporters sending to a local collector and +/// `sslmode=disable` behind the AlloyDB connector were seven wrong reviews, +/// and so were httpx's `verify=False` branch and vaultwarden's setting that +/// lets the operator accept invalid SMTP certificates. Asked whether code +/// turns verification off, gRPC's insecure credentials stayed reviews +/// whatever the examples said, as turning TLS off; asked whether code +/// accepts what a server presents without verifying it, a connection +/// without TLS presents nothing. i18next's `escapeValue: false` in a React +/// Native app was a wrong escape review. pub const WEAK_SETTINGS: [Check; 7] = [ Check { id: "tls", - question: "Does `{code}` turn off certificate or host name verification?", - yes: "It turns off certificate or host name checks, or accepts invalid certificates or host names.", - no: "It keeps verification on, or makes no TLS connection.", - no_examples: &[], + question: "Does `{code}` accept a server's certificate, host key or host name without verifying it?", + yes: "It turns off certificate, host key or host name checks, or accepts invalid ones, such as with `verify=False`, `InsecureSkipVerify`, `rejectUnauthorized: false`, `CERT_NONE` or a host key callback that accepts any key.", + no: "It verifies what the server presents; it connects without TLS, where no certificate is presented to verify; or it skips verification only when its caller or the operator asks for it, and verifies by default.", + no_examples: &[ + "A connection made without TLS, such as gRPC's `insecure.NewCredentials()` or `insecure_channel`, an OpenTelemetry exporter with `insecure=True`, or `sslmode=disable`", + "A library function that builds a TLS context without verification only when its caller passes `verify=False`, or a mail transport that accepts invalid certificates only when the operator turns on a setting for it", + ], }, Check { id: "hash", @@ -529,7 +543,9 @@ pub const WEAK_SETTINGS: [Check; 7] = [ question: "Does `{code}` turn off the automatic escaping of values written into HTML?", yes: "It turns off a template engine's or serializer's escaping of HTML for output that browsers render, such as autoescape set to false or escape_html_entities_in_json set to false.", no: "Escaping stays on; the output is not HTML that browsers render, such as Markdown, plain-text email or source code; or it configures no escaping.", - no_examples: &[], + no_examples: &[ + "i18next's `escapeValue: false` in a React or React Native app, whose components escape the translated text they render", + ], }, ]; From b7a0f2c884aee2648e248f47eafbb7634dcdbd1a Mon Sep 17 00:00:00 2001 From: Tauan BF <11513929+tauanbinato@users.noreply.github.com> Date: Sat, 26 Sep 2026 18:14:30 -0300 Subject: [PATCH 4/9] Measure the unreleased changes again, with 14 projects added after tuning --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c352b43..098dc09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ Notable changes to JevGate. Versions follow [Semantic Versioning](https://semver ## [Unreleased] -Measured on 103 pinned projects (24 new open-source ones of kinds not tried before, among them intentionally vulnerable Rails, Node, GraphQL, C# and Java apps, a Deno framework, a WordPress plugin, a cookiecutter template and projects in Kotlin, Swift, Elixir and C, and 8 more of the maintainer's own), with findings labeled by hand: on the 70 labeled projects JevGate was tuned on, 75% of reviews were right against 69% with 0.20.0 (136 wrong reviews against 192), and 72% of considers against 65% (254 wrong considers against 354); on 11 held-out projects, 61% of reviews against 57%, and 56% of considers against 54%. Undecided units went from 2.2% to 1.5% of judged units. +Measured on 103 pinned projects (24 new open-source ones of kinds not tried before, among them intentionally vulnerable Rails, Node, GraphQL, C# and Java apps, a Deno framework, a WordPress plugin, a cookiecutter template and projects in Kotlin, Swift, Elixir and C, and 8 more of the maintainer's own), with findings labeled by hand: on the 72 labeled projects JevGate was tuned on, 76% of reviews were right against 69% with 0.20.0 (142 wrong reviews against 192), and 73% of considers against 64% (239 wrong considers against 355); on 11 held-out projects, 63% of reviews against 57%, and 56% of considers against 54%. On 14 projects added after that tuning (9 of the maintainer's own, Online Boutique, a browser extension, a React Native template, a Solidity and a dbt project), 64% of reviews and 65% of considers were right, against 53% and 47%. Undecided units went from 2.2% to 1.5% of judged units. - Server templates: ERB, EJS, JSP, Handlebars, Mustache, Nunjucks, Twig, Jinja and Go templates, and HTML under `templates/`, `views/`, `layouts/`, `partials/` or `includes/`, are judged. Their inline `