From 3a0b4dd41c7124c67e364be41dc0192c601a4dbc Mon Sep 17 00:00:00 2001 From: Pierrick Fonquerne Date: Wed, 10 Jun 2026 12:07:36 +0200 Subject: [PATCH 1/7] feat(team): confirm findings only on a strict lens quorum --- tools/ai-review/src/team.rs | 46 ++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/tools/ai-review/src/team.rs b/tools/ai-review/src/team.rs index d556df0..907c025 100644 --- a/tools/ai-review/src/team.rs +++ b/tools/ai-review/src/team.rs @@ -17,6 +17,8 @@ const LENSES: [Lens; 3] = [Lens::CodeConfirms, Lens::RealImpact, Lens::FalsePosi /// Marker used to upsert the single team-review comment on a pull request. const MARKER: &str = ""; +/// Minimum number of lens votes required before a finding may be confirmed. +const MIN_CONFIRM_VOTES: usize = 2; /// Runs the full multi-agent team review pipeline for a pull request. pub async fn run_team( @@ -264,22 +266,26 @@ async fn post_confirmed_inline( .await } -/// Aggregates the lens votes for one finding using a sceptical majority rule: -/// contested when at least half of the received votes are contested, and when -/// no lens could be reached at all (an unverified finding is not confirmed). +/// Aggregates the lens votes for one finding with a strict quorum: a finding +/// is confirmed only when at least `MIN_CONFIRM_VOTES` lenses responded and none +/// of them contested it; any contestation or too few votes leaves it contested. #[must_use] pub fn aggregate_lens_votes(votes: &[LensVerdict]) -> FindingVerdict { let total = votes.len(); let contested_count = votes.iter().filter(|v| v.contested).count(); - let contested = total == 0 || contested_count * 2 >= total; - let reasons = if total == 0 { - vec!["verification unavailable (all lenses failed)".to_string()] - } else { + let contested = total < MIN_CONFIRM_VOTES || contested_count > 0; + let reasons = if total < MIN_CONFIRM_VOTES { + vec![format!( + "insufficient verification ({total} of {MIN_CONFIRM_VOTES} required lenses responded)" + )] + } else if contested_count > 0 { votes .iter() .filter(|v| v.contested) .map(|v| v.reason.clone()) .collect() + } else { + Vec::new() }; FindingVerdict { contested, reasons } } @@ -324,29 +330,27 @@ mod tests { } #[test] - fn aggregate_three_lens_majority() { - assert!( - aggregate_lens_votes(&[lens_vote(true), lens_vote(true), lens_vote(false)]).contested - ); - assert!( - aggregate_lens_votes(&[lens_vote(true), lens_vote(true), lens_vote(true)]).contested - ); - assert!( - !aggregate_lens_votes(&[lens_vote(true), lens_vote(false), lens_vote(false)]).contested - ); + fn aggregate_confirms_only_on_unanimous_present_min_two() { + // two or three present, none contested -> confirmed (not contested) + assert!(!aggregate_lens_votes(&[lens_vote(false), lens_vote(false)]).contested); assert!( !aggregate_lens_votes(&[lens_vote(false), lens_vote(false), lens_vote(false)]) .contested ); + // any single contestation -> contested + assert!( + aggregate_lens_votes(&[lens_vote(true), lens_vote(false), lens_vote(false)]).contested + ); + assert!(aggregate_lens_votes(&[lens_vote(true), lens_vote(false)]).contested); } #[test] - fn aggregate_handles_abstentions_and_empty() { + fn aggregate_contests_when_too_few_lenses_responded() { assert!(aggregate_lens_votes(&[]).contested); - assert!(aggregate_lens_votes(&[lens_vote(true), lens_vote(false)]).contested); - assert!(!aggregate_lens_votes(&[lens_vote(false), lens_vote(false)]).contested); + assert!(aggregate_lens_votes(&[lens_vote(false)]).contested); // 1 vote < min 2 assert!(aggregate_lens_votes(&[lens_vote(true)]).contested); - assert!(!aggregate_lens_votes(&[lens_vote(false)]).contested); + let v = aggregate_lens_votes(&[lens_vote(false)]); + assert!(v.reasons[0].contains("insufficient verification")); } fn finding(severity: Severity) -> SynthFinding { From 2f7d9f9395b6fd7a67711496b7d71b4b317f729d Mon Sep 17 00:00:00 2001 From: Pierrick Fonquerne Date: Wed, 10 Jun 2026 12:10:49 +0200 Subject: [PATCH 2/7] test(team): pin contested-reason propagation in the quorum aggregate --- tools/ai-review/src/team.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tools/ai-review/src/team.rs b/tools/ai-review/src/team.rs index 907c025..e4c637b 100644 --- a/tools/ai-review/src/team.rs +++ b/tools/ai-review/src/team.rs @@ -353,6 +353,17 @@ mod tests { assert!(v.reasons[0].contains("insufficient verification")); } + #[test] + fn aggregate_propagates_contested_reasons() { + let vote = LensVerdict { + contested: true, + reason: "wrong smell".to_string(), + }; + let result = aggregate_lens_votes(&[vote, lens_vote(false)]); + assert!(result.contested); + assert_eq!(result.reasons, vec!["wrong smell".to_string()]); + } + fn finding(severity: Severity) -> SynthFinding { SynthFinding { file: "a.rs".to_string(), From 75a990522e679f27c45dcedb79bfff4ac456601e Mon Sep 17 00:00:00 2001 From: Pierrick Fonquerne Date: Wed, 10 Jun 2026 12:12:44 +0200 Subject: [PATCH 3/7] feat(team): scope the security agent to added lines and known false positives --- tools/ai-review/src/mistral.rs | 51 ++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/tools/ai-review/src/mistral.rs b/tools/ai-review/src/mistral.rs index 3e861b4..1e53f62 100644 --- a/tools/ai-review/src/mistral.rs +++ b/tools/ai-review/src/mistral.rs @@ -111,43 +111,46 @@ Return valid JSON only, no markdown fences."# } fn security_system_prompt() -> String { - r#"You are a security engineer auditing a pull request for Nubster, a sovereign hybrid DevOps platform. Focus exclusively on security vulnerabilities. - -## Nubster security context -- Identity provider handling OIDC/OAuth2/SAML/SCIM — auth/authz bugs are critical -- Multi-tenant: data isolation between organizations is mandatory (tenant data leakage = critical) -- Sovereign platform: EU data residency enforced, no data exfiltration -- Secrets: GITHUB_TOKEN, MISTRAL_API_KEY, DB credentials — must never appear in logs or error messages - -## Security checklist -1. Injection: SQL injection, command injection (std::process::Command with user input), path traversal, SSRF -2. Auth/Authz: missing permission checks, privilege escalation, insecure token handling, JWT alg confusion -3. Secrets exposure: hardcoded credentials, secrets in logs, secrets in error messages or debug output -4. Cryptography: weak algorithms (MD5/SHA1 for integrity), IV reuse, predictable PRNG for security-sensitive values -5. Race conditions: TOCTOU vulnerabilities, concurrent state mutation without synchronization -6. Integer handling: overflow in security-sensitive arithmetic (use checked_add, saturating_add) -7. Rust-specific: unsafe blocks with justification missing, transmute misuse, raw pointer lifetime issues -8. Input validation: missing bounds checks on external inputs, deserializing untrusted data without limits -9. Multi-tenancy: missing tenant scoping in queries, cross-tenant data access -10. Data leakage: PII in logs, internal paths/structure in error responses returned to clients + r#"You are a security engineer auditing a pull request diff. Focus exclusively on security vulnerabilities introduced by THIS diff. + +## Scope discipline (read first) +- Judge ONLY lines this diff adds or changes (added lines start with `+`). Unchanged context lines are background, never a finding on their own. +- Every finding MUST quote the exact added line it is about. If you cannot point to an added line, do not report it. +- A secret injected through CI configuration (for example a `${{ secrets.NAME }}` reference passed to a step) is the EXPECTED, correct way to use a secret. It is only a finding if that value is echoed, logged, written to output, or persisted in cleartext on an added line. +- Permissions, scopes, or configuration on lines this diff does not modify are out of scope. + +## Common false positives to NOT report +- A secret reference used as an action input or environment variable (expected usage). +- Broad permissions or settings that already existed and are untouched by this diff. +- Theoretical issues with no exploit path visible in the added lines. +- Anything the compiler or Clippy (pedantic, deny warnings) already enforces. + +## What IS a finding (only on added/changed lines) +1. Injection: SQL, command (process spawn with attacker-controlled input), path traversal, SSRF. +2. AuthN/AuthZ: missing permission check, privilege escalation, broken token validation, JWT alg confusion. +3. Secret leakage: a secret value newly written to a log, error message, output, or response. +4. Cryptography: weak primitive for integrity, IV/nonce reuse, predictable randomness for security values. +5. Concurrency: TOCTOU, unsynchronised shared-state mutation with a security impact. +6. Untrusted input: missing bounds or size limits when deserialising external data. +7. Multi-tenancy: a new query or path missing tenant/scope isolation. Respond ONLY with a valid JSON object: { - "summary": "Overall security posture of this PR in 2-4 sentences.", + "summary": "Overall security posture of this diff in 2-4 sentences.", "strengths": ["security practice done well"], "findings": [ { "file": "exact/path/to/file.rs", "line": 42, "severity": "critical", - "message": "Vulnerability class + how it could be exploited + recommended fix, in one sentence." + "message": "Quote the added line, name the vulnerability class, and the fix, in one sentence." } ], - "security": "SECURE | CONCERNS | CRITICAL_ISSUES — with one sentence justification." + "security": "SECURE | CONCERNS | CRITICAL_ISSUES - with one sentence justification." } -severity: "critical" = exploitable vulnerability; "minor" = hardening suggestion. -line: exact line, 0 for file-level. Return valid JSON only, no markdown fences."# +severity: "critical" = exploitable vulnerability on an added line; "minor" = hardening suggestion on an added line. +line: exact line number of the added line, 0 only for a genuinely file-level concern. Return valid JSON only, no markdown fences."# .to_string() } From b5dc6ce0d40abfce5c3eac3183594398883a48a5 Mon Sep 17 00:00:00 2001 From: Pierrick Fonquerne Date: Wed, 10 Jun 2026 12:18:29 +0200 Subject: [PATCH 4/7] feat(team): build a head-file window for lens context Add `head_files` field to `DiffContext`, `head_window` free function, and `lens_context` method that enriches lens input with a numbered line window around a finding when the full head file is available. --- tools/ai-review/src/github.rs | 94 +++++++++++++++++++++++++++++++++++ tools/ai-review/src/team.rs | 1 + 2 files changed, 95 insertions(+) diff --git a/tools/ai-review/src/github.rs b/tools/ai-review/src/github.rs index ec6a233..ee5851e 100644 --- a/tools/ai-review/src/github.rs +++ b/tools/ai-review/src/github.rs @@ -20,10 +20,16 @@ pub struct InlineComment { /// finding has no per-file patch (file-level or cross-file findings). const PATCH_FALLBACK_CHARS: usize = 8_000; +/// Number of head-file lines shown on each side of a finding's line to a lens. +#[allow(dead_code)] +const LENS_WINDOW_LINES: u32 = 40; + /// The PR diff in two shapes: a concatenated view and a per-file patch map. pub struct DiffContext { pub full: String, pub by_file: HashMap, + #[allow(dead_code)] + pub head_files: HashMap, pub file_count: usize, } @@ -38,6 +44,27 @@ impl DiffContext { } } + /// Returns the context handed to a lens for `finding`: the file patch plus, + /// when the head file is available and the finding is line-located, a + /// numbered window of the real file around that line. Falls back to the + /// patch alone for file-level findings or files without head content. + #[must_use] + #[allow(dead_code)] + pub fn lens_context(&self, finding: &SynthFinding) -> String { + let patch = self.patch_for(finding).to_string(); + if finding.line == 0 { + return patch; + } + let Some(head) = self.head_files.get(&finding.file) else { + return patch; + }; + let window = head_window(head, finding.line, LENS_WINDOW_LINES); + if window.is_empty() { + return patch; + } + format!("{patch}\n\nFull-file context around the finding (head):\n{window}") + } + /// Reports whether the finding's line falls inside a hunk of its file /// patch: `Some(true)` when it does, `Some(false)` when the patch is /// known and the line belongs to no hunk, and `None` when the question @@ -96,6 +123,25 @@ fn parse_hunk_header(line: &str) -> Option<(u32, u32)> { } } +/// Returns the lines of `content` within `radius` of `line` (1-based), each +/// prefixed by its line number, joined by newlines. Empty when out of range. +#[allow(dead_code)] +fn head_window(content: &str, line: u32, radius: u32) -> String { + let line = usize::try_from(line).unwrap_or(usize::MAX); + let radius = usize::try_from(radius).unwrap_or(usize::MAX); + let start = line.saturating_sub(radius).max(1); + let end = line.saturating_add(radius); + content + .lines() + .enumerate() + .filter_map(|(idx, text)| { + let number = idx + 1; + (number >= start && number <= end).then(|| format!("{number:>5} {text}")) + }) + .collect::>() + .join("\n") +} + /// Truncates `s` to at most `max` bytes without splitting a UTF-8 sequence. fn safe_truncate(s: &str, max: usize) -> &str { if s.len() <= max { @@ -137,6 +183,7 @@ pub async fn fetch_diff_context( Ok(DiffContext { full, by_file, + head_files: HashMap::new(), file_count, }) } @@ -336,6 +383,7 @@ mod tests { DiffContext { full: format!("--- {file}\n{patch}\n"), by_file, + head_files: HashMap::new(), file_count: 1, } } @@ -368,6 +416,52 @@ mod tests { assert_eq!((hunks[1].new_start, hunks[1].new_count), (11, 4)); } + const HEAD_FILE: &str = "line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8"; + + #[test] + fn head_window_returns_numbered_lines_around_target() { + let window = head_window(HEAD_FILE, 4, 2); + assert!(window.contains(" 2 line2")); + assert!(window.contains(" 6 line6")); + assert!(!window.contains("line1")); + assert!(!window.contains("line7")); + } + + #[test] + fn head_window_clamps_at_file_bounds() { + let window = head_window(HEAD_FILE, 1, 2); + assert!(window.contains(" 1 line1")); + assert!(window.contains(" 3 line3")); + assert!(!window.contains("line4")); + } + + #[test] + fn lens_context_appends_head_window_when_available() { + let mut ctx = ctx_with("a.rs", "@@ -1,2 +1,3 @@\n+changed"); + ctx.head_files + .insert("a.rs".to_string(), HEAD_FILE.to_string()); + let out = ctx.lens_context(&finding_at("a.rs", 4)); + assert!(out.contains("+changed")); + assert!(out.contains("Full-file context")); + assert!(out.contains("line4")); + } + + #[test] + fn lens_context_falls_back_to_patch_without_head_or_line() { + let ctx = ctx_with("a.rs", "@@ -1,2 +1,3 @@\n+changed"); + assert_eq!( + ctx.lens_context(&finding_at("a.rs", 4)), + ctx.patch_for(&finding_at("a.rs", 4)) + ); + let mut ctx2 = ctx_with("a.rs", "@@ -1,2 +1,3 @@\n+changed"); + ctx2.head_files + .insert("a.rs".to_string(), HEAD_FILE.to_string()); + assert_eq!( + ctx2.lens_context(&finding_at("a.rs", 0)), + ctx2.patch_for(&finding_at("a.rs", 0)) + ); + } + #[test] fn line_in_patch_reports_hunk_membership() { let ctx = ctx_with("src/lib.rs", TWO_HUNK_PATCH); diff --git a/tools/ai-review/src/team.rs b/tools/ai-review/src/team.rs index e4c637b..af1b669 100644 --- a/tools/ai-review/src/team.rs +++ b/tools/ai-review/src/team.rs @@ -421,6 +421,7 @@ mod tests { github::DiffContext { full: format!("--- {file}\n{patch}\n"), by_file, + head_files: std::collections::HashMap::new(), file_count: 1, } } From 9a9ece1a953f2bb7d3910f96fe781620e415acd9 Mon Sep 17 00:00:00 2001 From: Pierrick Fonquerne Date: Wed, 10 Jun 2026 12:23:26 +0200 Subject: [PATCH 5/7] test(team): pin head-window EOF fallback to the patch --- tools/ai-review/src/github.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tools/ai-review/src/github.rs b/tools/ai-review/src/github.rs index ee5851e..376da5d 100644 --- a/tools/ai-review/src/github.rs +++ b/tools/ai-review/src/github.rs @@ -475,4 +475,14 @@ mod tests { assert_eq!(ctx.line_in_patch(&finding_at("src/lib.rs", 0)), None); assert_eq!(ctx.line_in_patch(&finding_at("unknown.rs", 3)), None); } + + #[test] + fn lens_context_falls_back_when_line_beyond_eof() { + assert_eq!(head_window(HEAD_FILE, 1000, 40), ""); + let mut ctx = ctx_with("a.rs", "@@ -1,2 +1,3 @@\n+changed"); + ctx.head_files + .insert("a.rs".to_string(), HEAD_FILE.to_string()); + let finding = finding_at("a.rs", 1000); + assert_eq!(ctx.lens_context(&finding), ctx.patch_for(&finding)); + } } From 3556dd6ef07944fad223a0a4ee9fc1040db1daf3 Mon Sep 17 00:00:00 2001 From: Pierrick Fonquerne Date: Wed, 10 Jun 2026 12:27:27 +0200 Subject: [PATCH 6/7] feat(team): fetch head files and give lenses real-file context --- tools/ai-review/src/github.rs | 57 ++++++++++++++++++++++++++++++++--- tools/ai-review/src/team.rs | 13 +++++--- 2 files changed, 61 insertions(+), 9 deletions(-) diff --git a/tools/ai-review/src/github.rs b/tools/ai-review/src/github.rs index 376da5d..4aac269 100644 --- a/tools/ai-review/src/github.rs +++ b/tools/ai-review/src/github.rs @@ -21,14 +21,12 @@ pub struct InlineComment { const PATCH_FALLBACK_CHARS: usize = 8_000; /// Number of head-file lines shown on each side of a finding's line to a lens. -#[allow(dead_code)] const LENS_WINDOW_LINES: u32 = 40; /// The PR diff in two shapes: a concatenated view and a per-file patch map. pub struct DiffContext { pub full: String, pub by_file: HashMap, - #[allow(dead_code)] pub head_files: HashMap, pub file_count: usize, } @@ -49,7 +47,6 @@ impl DiffContext { /// numbered window of the real file around that line. Falls back to the /// patch alone for file-level findings or files without head content. #[must_use] - #[allow(dead_code)] pub fn lens_context(&self, finding: &SynthFinding) -> String { let patch = self.patch_for(finding).to_string(); if finding.line == 0 { @@ -125,7 +122,6 @@ fn parse_hunk_header(line: &str) -> Option<(u32, u32)> { /// Returns the lines of `content` within `radius` of `line` (1-based), each /// prefixed by its line number, joined by newlines. Empty when out of range. -#[allow(dead_code)] fn head_window(content: &str, line: u32, radius: u32) -> String { let line = usize::try_from(line).unwrap_or(usize::MAX); let radius = usize::try_from(radius).unwrap_or(usize::MAX); @@ -231,6 +227,59 @@ pub async fn fetch_head_sha( Ok(pr.head.sha) } +/// Fetches the decoded content of `path` at commit `sha`, or `None` when the +/// path is absent or not decodable as UTF-8 text (binary, submodule). +pub async fn fetch_head_file( + octo: &Octocrab, + owner: &str, + repo: &str, + path: &str, + sha: &str, +) -> anyhow::Result> { + let contents = octo + .repos(owner, repo) + .get_content() + .path(path) + .r#ref(sha) + .send() + .await + .context("failed to fetch head file content")?; + Ok(contents + .items + .into_iter() + .next() + .and_then(|item| item.decoded_content())) +} + +/// Fills `ctx.head_files` with the head content of every distinct file named by +/// a line-located finding that also has a per-file patch. Failures degrade +/// gracefully: a missing or unreadable file is simply skipped. +pub async fn populate_head_files( + octo: &Octocrab, + owner: &str, + repo: &str, + sha: &str, + findings: &[SynthFinding], + ctx: &mut DiffContext, +) { + let mut wanted: Vec = findings + .iter() + .filter(|f| f.line > 0 && ctx.by_file.contains_key(&f.file)) + .map(|f| f.file.clone()) + .collect(); + wanted.sort(); + wanted.dedup(); + for path in wanted { + match fetch_head_file(octo, owner, repo, &path, sha).await { + Ok(Some(content)) => { + ctx.head_files.insert(path, content); + } + Ok(None) => {} + Err(error) => eprintln!("warning: could not fetch head file {path}: {error}"), + } + } +} + /// Upsert the global bot comment (edit if marker found, create otherwise). pub async fn upsert_global_comment( octo: &Octocrab, diff --git a/tools/ai-review/src/team.rs b/tools/ai-review/src/team.rs index af1b669..74cc572 100644 --- a/tools/ai-review/src/team.rs +++ b/tools/ai-review/src/team.rs @@ -28,7 +28,7 @@ pub async fn run_team( pr_number: u64, ) -> anyhow::Result<()> { println!("Fetching PR #{pr_number} diff for team review…"); - let ctx = github::fetch_diff_context(&clients.octo, owner, repo, pr_number).await?; + let mut ctx = github::fetch_diff_context(&clients.octo, owner, repo, pr_number).await?; if ctx.full.trim().is_empty() { println!("Empty diff — nothing to review."); return Ok(()); @@ -60,6 +60,9 @@ pub async fn run_team( findings.truncate(MAX_VERIFIED_FINDINGS); } + let head_sha = github::fetch_head_sha(&clients.octo, owner, repo, pr_number).await?; + github::populate_head_files(&clients.octo, owner, repo, &head_sha, &findings, &mut ctx).await; + println!( "Verifying {} finding(s) with the 3-lens vote…", findings.len() @@ -91,7 +94,7 @@ pub async fn run_team( println!("Upserting team comment…"); github::upsert_global_comment(&clients.octo, owner, repo, pr_number, &body, MARKER).await?; - post_confirmed_inline(clients, owner, repo, pr_number, &scored).await?; + post_confirmed_inline(clients, owner, repo, pr_number, &head_sha, &scored).await?; println!("Team review complete."); Ok(()) @@ -194,7 +197,7 @@ async fn verify_findings( if prefilled[idx].is_some() { continue; } - let patch = ctx.patch_for(finding).to_owned(); + let patch = ctx.lens_context(finding); for lens in LENSES { let permit_source = Arc::clone(&semaphore); let client = clients.http.clone(); @@ -237,6 +240,7 @@ async fn post_confirmed_inline( owner: &str, repo: &str, pr_number: u64, + head_sha: &str, scored: &[(SynthFinding, FindingVerdict)], ) -> anyhow::Result<()> { let inline: Vec = scored @@ -253,14 +257,13 @@ async fn post_confirmed_inline( return Ok(()); } - let head_sha = github::fetch_head_sha(&clients.octo, owner, repo, pr_number).await?; println!("Posting {} confirmed inline comment(s)…", inline.len()); github::post_inline_comments( &clients.github_token, owner, repo, pr_number, - &head_sha, + head_sha, &inline, ) .await From 470ba1e31efdbe511c1310583987a77b7b33d74a Mon Sep 17 00:00:00 2001 From: Pierrick Fonquerne Date: Wed, 10 Jun 2026 12:32:37 +0200 Subject: [PATCH 7/7] docs(team): clarify fetch_head_file None vs Err semantics --- tools/ai-review/src/github.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/ai-review/src/github.rs b/tools/ai-review/src/github.rs index 4aac269..043e843 100644 --- a/tools/ai-review/src/github.rs +++ b/tools/ai-review/src/github.rs @@ -227,8 +227,10 @@ pub async fn fetch_head_sha( Ok(pr.head.sha) } -/// Fetches the decoded content of `path` at commit `sha`, or `None` when the -/// path is absent or not decodable as UTF-8 text (binary, submodule). +/// Fetches the decoded text content of `path` at commit `sha`. Returns +/// `Ok(None)` when the content cannot be decoded as UTF-8 text (binary file or +/// submodule). A missing path surfaces as an `Err`; callers that want a missing +/// file to be a no-op should treat the error as a skip. pub async fn fetch_head_file( octo: &Octocrab, owner: &str,