Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ Notable changes to JevGate. Versions follow [Semantic Versioning](https://semver

## [Unreleased]

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.
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.

- Function simplification: splitting a function of 20 lines or fewer is at most a note. 12 of 39 such considers were right on the tuned projects and 5 of 50 on the blind Bend 2 ones: helpers that read in one look, a dispatch over a token's cases, proofs. Function-simplification considers went from 68% to 73% right. Nothing is asked again.
- Shared logic: copies of up to twelve lines between test cases in different files are notes, like short copies inside test cases: tests of separate modules or rules repeat the same setup because the code they test is parallel, and a helper shared across test files would couple them. 30 of 87 such considers were right on the tuned projects and 2 of 24 on the held-out ones; shared-logic considers went from 53% to 57% right there and from 43% to 53% on the held-out projects. Nothing is asked again.
- File organization: a split the recheck raised from an undecided first answer is asked what kind of file it is, as an undecided one is, and a kind that serves one feature clears it. The first pass and the recheck disagreeing was a weak sign: 4 of the 18 findings it cleared were right. About $0.01 on the corpus.
- Injection: a finding that rests on a file path is asked what the path's variable parts can hold, with its callers and the definitions of the project's types its parameters name (a Rust derive list such as `#[derive(UuidFromParam)]`); leaning toward names that stay inside their directory (an id its type parses, a base name, a Rocket `PathBuf` route segment, which rejects hidden and encoded-slash segments), it is a note. vaultwarden's four path reviews on typed Rocket route parameters were all wrong and are notes; the five path findings labeled right answered another party's input at 0.96 or more and stay.
- Sensitive data: a finding its log checks raised is asked when the log line runs; a line that runs only when an operator turns on a setting meant for logging those values, off by default, is a note. vaultwarden's SSO tokens under `SSO_DEBUG_TOKENS` and pgweb's opt-in request logger, both labeled wrong, are notes; every log finding labeled right answered normal operation or debug level and stays. The two changes cleared 6 wrong security reviews and no right one, for under $0.01 on the corpus.

## [0.22.0] - 2026-09-27

Expand Down
4 changes: 2 additions & 2 deletions src/catalog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,8 @@ pub fn rule_version(key: &str) -> &'static str {
SHARED_LOGIC => "22",
TEST_VALUE => "7",
TEST_REDUNDANCY => "4",
INJECTION => "10",
SENSITIVE_DATA => "7",
INJECTION => "11",
SENSITIVE_DATA => "8",
HARDCODED_VALUES => "8",
UNSAFE_SETTINGS => "6",
AGENT_CONTEXT => "3",
Expand Down
112 changes: 70 additions & 42 deletions src/units/compose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use super::{
Access, Block, Detail, FilePlan, Presence, UnitPlan,
outcome::{
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,
logs_found, lowered, noul, open, origin_outcome, rests_on_paths, score, settled_checks,
several_kind, unit_outcome, value_signals,
},
wording::{Wording, comment_reason, comment_wording},
wording::{
Expand Down Expand Up @@ -249,57 +249,85 @@ fn rechecked<'a>(unit: &UnitPlan, judgments: &'a [Judgment]) -> (Outcome, Answer
(outcome, first)
}

/// 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.
pub fn unlocated_units(plan: &FilePlan, judgments: &[Judgment]) -> BTreeSet<String> {
/// 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).
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 {
return false;
};
let (outcome, resolved) = resolved(u, judgments);
let raised =
|o: Option<Outcome>| matches!(o, Some(Outcome::Review(_) | Outcome::Consider(_)));
match &u.detail {
Detail::Values {
locate: Some(_), ..
}
| Detail::Constants {
locate: Some(_), ..
} => raised(Some(outcome)),
Detail::TestPair {
confirm: Some(_), ..
} => 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))),
Detail::Document {
locate: Some(_), ..
} => raised(
resolved
.get("split")
.map(|a| document_split(a, resolved.get("kind").copied())),
),
_ => false,
}
let get = |q: &str| resolved.get(q).copied();
matches!(outcome, Outcome::Review(_) | Outcome::Consider(_))
&& (paths.is_some() && rests_on_paths(&get)
|| logging.is_some() && logs_found(&get))
})
.map(|u| u.id.clone())
.collect()
}

/// 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
/// units whose confirm `locate_due` calls for.
pub fn unlocated_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| locate_due(u, judgments))
.map(|u| u.id.clone())
.collect()
}

/// Whether a unit's outcome calls for its locate or confirm follow-up.
fn locate_due(unit: &UnitPlan, judgments: &[Judgment]) -> bool {
let (outcome, resolved) = resolved(unit, judgments);
let raised = |o: Option<Outcome>| matches!(o, Some(Outcome::Review(_) | Outcome::Consider(_)));
match &unit.detail {
Detail::Values {
locate: Some(_), ..
}
| Detail::Constants {
locate: Some(_), ..
} => raised(Some(outcome)),
Detail::TestPair {
confirm: Some(_), ..
} => 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; one that rests on a path is asked
// what its paths can hold instead.
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())
}
Detail::Function {
locate: Some(_), ..
} => raised(resolved.get("split").map(|a| benefit(a))),
Detail::Document {
locate: Some(_), ..
} => raised(
resolved
.get("split")
.map(|a| document_split(a, resolved.get("kind").copied())),
),
_ => false,
}
}

/// Judged units whose first pass stayed undecided, or became a note from a
/// torn function or file-organization answer, and that have no recheck yet;
/// for injection, units whose traced origin stayed unclear or was the
Expand Down
26 changes: 20 additions & 6 deletions src/units/follow_ups.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,24 @@ 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, per test that asserts internal details, and
/// per injection consider that rests on its parameters.
/// test pair raised to a review, per test that asserts internal details, per
/// injection consider that rests on its parameters, per injection finding
/// that rests on a path and per logging finding.
pub fn locates(plan: &Plan, files: &[FileResult]) -> Vec<Planned> {
follow_ups(plan, files, compose::unlocated_units, |unit| {
match &unit.detail {
let mut planned = follow_ups(
plan,
files,
compose::unconfirmed_units,
|unit| match &unit.detail {
Detail::Security { paths, logging, .. } => paths.as_ref().or(logging.as_ref()),
_ => None,
},
);
planned.extend(follow_ups(
plan,
files,
compose::unlocated_units,
|unit| match &unit.detail {
Detail::Function { locate, .. }
| Detail::Document { locate, .. }
| Detail::Values { locate, .. }
Expand All @@ -20,8 +33,9 @@ pub fn locates(plan: &Plan, files: &[FileResult]) -> Vec<Planned> {
| Detail::Test { confirm }
| Detail::Security { confirm, .. } => confirm.as_ref(),
_ => None,
}
})
},
));
planned
}

/// One question per hardcoded-value consider resting on a value's name
Expand Down
6 changes: 6 additions & 0 deletions src/units/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,12 @@ 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 sensitive data, when its log line runs, asked only after a
/// finding its log checks raised.
logging: Option<FollowUp>,
/// Django code, asked the Django checks: a weak setting must be
/// named by one of them.
django: bool,
Expand Down
44 changes: 43 additions & 1 deletion src/units/outcome/exposure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,27 @@ pub(in crate::units) fn messages<'a>(
})
}

/// The sensitive-data signals that a function writes a value to a log.
pub(in crate::units) const LOG_SIGNALS: [&str; 2] = ["logs_secret", "logs_object_secret"];

/// Whether a log signal reached a finding: its log line is then asked when
/// it runs.
pub(in crate::units) fn logs_found<'a>(get: &impl Fn(&str) -> Option<&'a Answer>) -> bool {
LOG_SIGNALS
.iter()
.any(|q| matches!(get(q).map(noul), Some(Outcome::Review(_))))
}

/// Whether the log line runs only when an operator turns on a setting whose
/// purpose is that logging, at the threshold. The operator chose to log the
/// value, so its log signals are at most a note. On the corpus it cleared
/// vaultwarden's SSO tokens under `SSO_DEBUG_TOKENS` and pgweb's request
/// logger, both labeled wrong; every log finding labeled right was asked it
/// and answered normal operation or debug level.
pub(in crate::units) fn opted_in<'a>(get: &impl Fn(&str) -> Option<&'a Answer>) -> bool {
choice_mass(get("logged_when"), &[questions::OPT_IN_LOGGING]).is_some_and(at_least)
}

/// A judged exposure answer and how far it leans toward its concern.
type Signal = (Outcome, f64);

Expand Down Expand Up @@ -91,7 +112,18 @@ fn exposure_signals<'a>(
.then(|| messages(get))
.flatten();
let away = rule == catalog::SENSITIVE_DATA && away_from_clients(get);
let judge = |question: &str, answer: &Answer| exposure_signal(question, answer, own, away);
let opted_in = rule == catalog::SENSITIVE_DATA && opted_in(get);
let judge = |question: &str, answer: &Answer| {
let signal = exposure_signal(question, answer, own, away);
match signal {
(Outcome::Review(p) | Outcome::Consider(p), lean)
if opted_in && LOG_SIGNALS.contains(&question) =>
{
(Outcome::Note(p), lean)
}
signal => signal,
}
};
// A Choice asked whenever a presence signal is not clear rules it out
// too: what a function's logs write clears an audit line that names who
// signed in, which the presence question found as personal data.
Expand Down Expand Up @@ -155,10 +187,20 @@ fn exposure_level(rule: &str, presence: &[Signal], specific: &[Signal]) -> Outco
!probability_at_least(*lean, LEADING_PROBABILITY)
&& !matches!(o, Outcome::Review(_) | Outcome::Consider(_))
});
// A signal already lowered to a note, such as a log line an operator
// turned on to log that value.
let noted = presence
.iter()
.chain(specific)
.filter(|(o, _)| matches!(o, Outcome::Note(_)))
.map(|(o, _)| o.concern())
.reduce(f64::max);
if unnamed && !found.is_empty() {
Outcome::Note(strongest(&found).concern())
} else if !found.is_empty() {
strongest(&found)
} else if let Some(p) = noted {
Outcome::Note(p)
} else if ruled_out {
Outcome::Clear
} else if probability_at_least(leaning, LEADING_PROBABILITY) {
Expand Down
23 changes: 23 additions & 0 deletions src/units/outcome/injection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,33 @@ 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 => 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 {
let found = found_injections(get);
!found.is_empty() && found.iter().all(|id| *id == "path")
}

/// 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))
}

/// 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
Expand Down
7 changes: 5 additions & 2 deletions src/units/outcome/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,12 @@ pub(super) use comments::{comment_concern_kind, comment_outcome, comment_signals
use documentation::staleness_outcome;
pub(super) use documentation::{document_outcome, document_split, section_signals};
pub(super) use exposure::{
Messages, django_settings_outcome, exposure_outcome, messages, settings_module_outcome,
Messages, django_settings_outcome, exposure_outcome, logs_found, messages, opted_in,
settings_module_outcome,
};
pub(super) use injection::{
RESOURCE_CHECKS, confined_paths, injection_outcome, origin_outcome, rests_on_paths,
};
pub(super) use injection::{RESOURCE_CHECKS, injection_outcome, origin_outcome};
pub(super) use maintainability::{
benign_key, function_outcome, organization_outcome, several_kind, shared_outcome,
value_signals, values_outcome,
Expand Down
9 changes: 7 additions & 2 deletions src/units/plan/security_units.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,13 @@ fn function_subject<'a>(
} else {
Vec::new()
};
let mut subject =
security::function_subject(context, unit, callers, &shared.enums, &shared.constants);
let mut subject = security::function_subject(
context,
unit,
callers,
(&shared.enums, &shared.types),
&shared.constants,
);
if rules.contains(&catalog::SENSITIVE_DATA) {
subject.callee_errors = callee_errors(scope, &shared.links, context.owner, unit);
}
Expand Down
5 changes: 4 additions & 1 deletion src/units/plan/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//! subjects and their sources, routes, helpers, enums, constants and hashes.
use super::{
Scope, java,
trace_evidence::{csharp_constants, enums},
trace_evidence::{csharp_constants, enums, types},
};
use crate::{
analysis::{
Expand Down Expand Up @@ -45,6 +45,7 @@ pub(super) struct Shared<'a> {
pub(super) cases: BTreeMap<PathBuf, Vec<TestCase>>,
/// Enum definitions by name, from selected files and context, for security traces.
pub(super) enums: BTreeMap<String, String>,
pub(super) types: BTreeMap<String, String>,
/// C# constants by field name, as `Class.Field = value`, for security traces.
pub(super) constants: BTreeMap<String, Vec<String>>,
pub(super) hashes: BTreeMap<PathBuf, String>,
Expand Down Expand Up @@ -117,6 +118,7 @@ impl<'a> Shared<'a> {
module_helpers: BTreeMap::new(),
cases,
enums: BTreeMap::new(),
types: BTreeMap::new(),
constants: BTreeMap::new(),
hashes: source_hashes(scope),
teaching: false,
Expand All @@ -138,6 +140,7 @@ impl<'a> Shared<'a> {
}
if shared.enabled(catalog::INJECTION) {
shared.enums = enums(scope);
shared.types = types(scope);
}
if shared.enabled(catalog::UNSAFE_SETTINGS) {
shared.constants = csharp_constants(scope);
Expand Down
Loading
Loading