From de181634cb3825754f05358096d9e7e58caaf4fb Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Thu, 7 Aug 2025 16:13:57 -0700 Subject: [PATCH 1/6] Fixed GitHub organization and GitLab group scans when using '--git-history=none' --- CHANGELOG.md | 3 +++ data/rules/onepassword.yml | 1 - src/cli/commands/github.rs | 2 +- src/cli/commands/inputs.rs | 4 ++-- src/gitlab.rs | 40 ++++++++++++++++++++++++++++++------- src/main.rs | 4 ++-- src/reporter/json_format.rs | 4 ++-- 7 files changed, 43 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 482eca5..b06639d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ All notable changes to this project will be documented in this file. +## [1.36.0] +- Fixed GitHub organization and GitLab group scans when using `--git-history=none` + ## [1.35.0] - Remote scans with `--git-history=none` now clone repositories with a working tree and scan the current files instead of erroring with "No inputs to scan". - Fixed issue where `--redact` did not function properly diff --git a/data/rules/onepassword.yml b/data/rules/onepassword.yml index e521e01..b7cc0bc 100644 --- a/data/rules/onepassword.yml +++ b/data/rules/onepassword.yml @@ -44,7 +44,6 @@ rules: \b min_entropy: 3.8 confidence: medium - prevalidated: true examples: - A3-R69SQK-TZ9KPW-8MXYD-6W373-V7GHJ-EDJQW - A3-ASWWYB-798JRY-LJVD4-23DC2-86TVM-H43EB diff --git a/src/cli/commands/github.rs b/src/cli/commands/github.rs index 766df83..cea9a44 100644 --- a/src/cli/commands/github.rs +++ b/src/cli/commands/github.rs @@ -60,7 +60,7 @@ pub struct GitHubRepoSpecifiers { pub all_organizations: bool, /// Filter by repository type - #[arg(long, default_value_t = GitHubRepoType::Source, alias = "github-repo-type")] + #[arg(long, default_value_t = GitHubRepoType::All, alias = "github-repo-type")] pub repo_type: GitHubRepoType, } diff --git a/src/cli/commands/inputs.rs b/src/cli/commands/inputs.rs index 13bc78b..4cf4f26 100644 --- a/src/cli/commands/inputs.rs +++ b/src/cli/commands/inputs.rs @@ -60,7 +60,7 @@ pub struct InputSpecifierArgs { )] pub github_api_url: Url, - #[arg(long, default_value_t = GitHubRepoType::Source)] + #[arg(long, default_value_t = GitHubRepoType::All)] pub github_repo_type: GitHubRepoType, // GitLab Options @@ -85,7 +85,7 @@ pub struct InputSpecifierArgs { )] pub gitlab_api_url: Url, - #[arg(long, default_value_t = GitLabRepoType::Owner)] + #[arg(long, default_value_t = GitLabRepoType::All)] pub gitlab_repo_type: GitLabRepoType, /// Jira base URL (e.g. https://jira.example.com) diff --git a/src/gitlab.rs b/src/gitlab.rs index c7b0549..d94f46c 100644 --- a/src/gitlab.rs +++ b/src/gitlab.rs @@ -88,9 +88,25 @@ pub async fn enumerate_repo_urls( hits.into_iter().next().context(format!("GitLab user `{}` not found", username))?; let user_id = user.id; - // b) List that user’s projects by ID - let projects_ep = UserProjects::builder().user(user_id).build()?; + // b) List that user's projects applying the requested filter + let mut builder = UserProjects::builder(); + builder.user(user_id); + + match repo_specifiers.repo_filter { + RepoType::Owner => { + builder.owned(true); + } + RepoType::Member => { + builder.membership(true); + } + RepoType::All => { + // nothing + } + } + + let projects_ep = builder.build()?; // now no borrows of a temporary let projects: Vec = projects_ep.query(&client)?; + for proj in projects { repo_urls.push(proj.http_url_to_repo); } @@ -102,19 +118,29 @@ pub async fn enumerate_repo_urls( // all groups let groups: Vec = if repo_specifiers.all_groups { - gitlab::api::groups::Groups::builder().build()?.query(&client.clone())? + gitlab::api::groups::Groups::builder() + .all_available(true) + .build()? + .query(&client.clone())? } else { let mut found: Vec = Vec::new(); for grp in &repo_specifiers.group { - let ep = gitlab::api::groups::Groups::builder().search(grp).build()?; - let page: Vec = ep.query(&client.clone())?; - found.extend(page); + let ep = gitlab::api::groups::Group::builder().group(grp).build()?; + let group: SimpleGroup = ep.query(&client.clone())?; + found.push(group); } found }; for group in groups { - let gp_ep = GroupProjects::builder().group(group.id).build()?; + let mut gp_builder = GroupProjects::builder(); + gp_builder.group(group.id); + + if matches!(repo_specifiers.repo_filter, RepoType::Owner) { + gp_builder.owned(true); + } + + let gp_ep = gp_builder.build()?; let projects: Vec = gp_ep.query(&client)?; for proj in projects { repo_urls.push(proj.http_url_to_repo); diff --git a/src/main.rs b/src/main.rs index 73c77a5..9c9c1bd 100644 --- a/src/main.rs +++ b/src/main.rs @@ -275,13 +275,13 @@ fn create_default_scan_args() -> cli::commands::scan::ScanArgs { github_organization: Vec::new(), all_github_organizations: false, github_api_url: url::Url::parse("https://api.github.com/").unwrap(), - github_repo_type: GitHubRepoType::Source, + github_repo_type: GitHubRepoType::All, // new GitLab defaults gitlab_user: Vec::new(), gitlab_group: Vec::new(), all_gitlab_groups: false, gitlab_api_url: Url::parse("https://gitlab.com/").unwrap(), - gitlab_repo_type: GitLabRepoType::Owner, + gitlab_repo_type: GitLabRepoType::All, jira_url: None, jql: None, diff --git a/src/reporter/json_format.rs b/src/reporter/json_format.rs index 154bb58..0e5a845 100644 --- a/src/reporter/json_format.rs +++ b/src/reporter/json_format.rs @@ -76,14 +76,14 @@ mod tests { github_organization: Vec::new(), all_github_organizations: false, github_api_url: Url::parse("https://api.github.com/").unwrap(), - github_repo_type: GitHubRepoType::Source, + github_repo_type: GitHubRepoType::All, // GitLab gitlab_user: Vec::new(), gitlab_group: Vec::new(), all_gitlab_groups: false, gitlab_api_url: Url::parse("https://gitlab.com/").unwrap(), - gitlab_repo_type: GitLabRepoType::Owner, + gitlab_repo_type: GitLabRepoType::All, // Jira options jira_url: None, jql: None, From b71fb5e6e2eb5c73bc53ffc46a2aee463a896296 Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Thu, 7 Aug 2025 17:21:16 -0700 Subject: [PATCH 2/6] JWT tokens without both 'iss' and 'aud' are no longer reported as active credentials --- CHANGELOG.md | 1 + data/rules/jira.yml | 2 +- src/scanner/validation.rs | 6 -- src/validation/jwt.rs | 4 ++ tests/int_rules_no_validated_findings.rs | 79 ++++++++++++++++++++++++ 5 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 tests/int_rules_no_validated_findings.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index b06639d..d6bc09d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ All notable changes to this project will be documented in this file. ## [1.36.0] - Fixed GitHub organization and GitLab group scans when using `--git-history=none` +- JWT tokens without both `iss` and `aud` are no longer reported as active credentials ## [1.35.0] - Remote scans with `--git-history=none` now clone repositories with a working tree and scan the current files instead of erroring with "No inputs to scan". diff --git a/data/rules/jira.yml b/data/rules/jira.yml index 82ac195..a555d85 100644 --- a/data/rules/jira.yml +++ b/data/rules/jira.yml @@ -11,7 +11,7 @@ rules: visible: false confidence: medium examples: - - example-jira.atlassian.net + - examplefoo-jira.atlassian.net - jira.sprintUri= https://example.atlassian.net/rest - name: Jira Token diff --git a/src/scanner/validation.rs b/src/scanner/validation.rs index 210ef26..1ba02e1 100644 --- a/src/scanner/validation.rs +++ b/src/scanner/validation.rs @@ -335,12 +335,6 @@ pub async fn run_secret_validation( ds.replace_matches(updated_arcs); } - // ── 5. Done ───────────────────────────────────────────────────────────── - println!( - "Validation complete – {} succeeded, {} failed", - success_count.load(Ordering::Relaxed), - fail_count.load(Ordering::Relaxed) - ); Ok(()) } diff --git a/src/validation/jwt.rs b/src/validation/jwt.rs index 25a7206..d5485d6 100644 --- a/src/validation/jwt.rs +++ b/src/validation/jwt.rs @@ -71,7 +71,11 @@ pub async fn validate_jwt(token: &str) -> Result<(bool, String)> { // --------------------------------------------------------------------------- let issuer = claims.iss.clone().unwrap_or_default(); + let aud_strings = extract_aud_strings(&claims); + if issuer.trim().is_empty() && aud_strings.iter().all(|s| s.trim().is_empty()) { + return Ok((false, "JWT missing issuer and audience".to_string())); + } if let Some(iss) = claims.iss.clone() { // parse header now (kid, alg) let header = decode_header(token).map_err(|e| anyhow!("decode header: {e}"))?; diff --git a/tests/int_rules_no_validated_findings.rs b/tests/int_rules_no_validated_findings.rs new file mode 100644 index 0000000..51d4a3b --- /dev/null +++ b/tests/int_rules_no_validated_findings.rs @@ -0,0 +1,79 @@ +use anyhow::Result; +use assert_cmd::Command; +use serde_json::Value; + +#[test] +fn scan_rules_has_no_validated_findings() -> Result<()> { + let output = Command::cargo_bin("kingfisher")? + .args([ + "scan", "data/rules", + "--format", "json", + "--no-update-check", + "--only-valid", + ]) + .output()?; + + let stdout = String::from_utf8_lossy(&output.stdout); + + // Find the first '[' — start of array + let start = match stdout.find('[') { + Some(i) => i, + None => return Ok(()), // no array found + }; + + let mut depth = 0usize; + let mut end = None; + for (i, ch) in stdout.char_indices().skip(start) { + match ch { + '[' => depth += 1, + ']' => { + depth -= 1; + if depth == 0 { + end = Some(i); + break; + } + } + _ => {} + } + } + + let json_array_str = match end { + Some(end_idx) => &stdout[start..=end_idx], + None => return Ok(()), // no matching close found + }; + + if json_array_str.trim().is_empty() { + return Ok(()); + } + + let findings: Vec = serde_json::from_str(json_array_str)?; + + for finding in findings { + let rule_id = finding["rule"]["id"].as_str().unwrap_or("unknown"); + let rule_prevalidated = finding["rule"]["prevalidated"].as_bool().unwrap_or(false); + + let status = finding["finding"]["validation"]["status"] + .as_str() + .unwrap_or("") + .to_ascii_lowercase(); + + let response = finding["finding"]["validation"]["response"] + .as_str() + .unwrap_or("") + .to_ascii_lowercase(); + + // Skip anything intentionally marked as prevalidated + if rule_prevalidated || status == "prevalidated" || response == "prevalidated" { + continue; + } + + // Fail only on genuinely validated secrets + assert_ne!( + status.as_str(), + "active credential", + "Validated finding detected in rule {rule_id}" + ); + } + + Ok(()) +} From 061d3d97ed3d35a2704a8c36d08abd7680cc131e Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Thu, 7 Aug 2025 17:21:31 -0700 Subject: [PATCH 3/6] JWT tokens without both 'iss' and 'aud' are no longer reported as active credentials --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 930a196..4114e10 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ publish = false [package] name = "kingfisher" -version = "1.35.0" +version = "1.36.0" description = "MongoDB's blazingly fast secret scanning and validation tool" edition.workspace = true rust-version.workspace = true From b3ddc119a4127ff5004388d7254dc5de3d2d98ea Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Thu, 7 Aug 2025 17:36:39 -0700 Subject: [PATCH 4/6] JWT tokens without both 'iss' and 'aud' are no longer reported as active credentials --- src/cli/commands/github.rs | 4 ++-- src/cli/commands/inputs.rs | 2 +- src/main.rs | 2 +- src/reporter/json_format.rs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cli/commands/github.rs b/src/cli/commands/github.rs index cea9a44..50aa190 100644 --- a/src/cli/commands/github.rs +++ b/src/cli/commands/github.rs @@ -60,7 +60,7 @@ pub struct GitHubRepoSpecifiers { pub all_organizations: bool, /// Filter by repository type - #[arg(long, default_value_t = GitHubRepoType::All, alias = "github-repo-type")] + #[arg(long, default_value_t = GitHubRepoType::Source, alias = "github-repo-type")] pub repo_type: GitHubRepoType, } @@ -87,7 +87,7 @@ pub enum GitHubRepoType { impl From for crate::github::RepoType { fn from(val: GitHubRepoType) -> Self { match val { - GitHubRepoType::All => crate::github::RepoType::All, + GitHubRepoType::Source => crate::github::RepoType::All, GitHubRepoType::Source => crate::github::RepoType::Source, GitHubRepoType::Fork => crate::github::RepoType::Fork, } diff --git a/src/cli/commands/inputs.rs b/src/cli/commands/inputs.rs index 4cf4f26..2249640 100644 --- a/src/cli/commands/inputs.rs +++ b/src/cli/commands/inputs.rs @@ -60,7 +60,7 @@ pub struct InputSpecifierArgs { )] pub github_api_url: Url, - #[arg(long, default_value_t = GitHubRepoType::All)] + #[arg(long, default_value_t = GitHubRepoType::Source)] pub github_repo_type: GitHubRepoType, // GitLab Options diff --git a/src/main.rs b/src/main.rs index 9c9c1bd..58145e6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -275,7 +275,7 @@ fn create_default_scan_args() -> cli::commands::scan::ScanArgs { github_organization: Vec::new(), all_github_organizations: false, github_api_url: url::Url::parse("https://api.github.com/").unwrap(), - github_repo_type: GitHubRepoType::All, + github_repo_type: GitHubRepoType::Source, // new GitLab defaults gitlab_user: Vec::new(), gitlab_group: Vec::new(), diff --git a/src/reporter/json_format.rs b/src/reporter/json_format.rs index 0e5a845..aae16fc 100644 --- a/src/reporter/json_format.rs +++ b/src/reporter/json_format.rs @@ -76,7 +76,7 @@ mod tests { github_organization: Vec::new(), all_github_organizations: false, github_api_url: Url::parse("https://api.github.com/").unwrap(), - github_repo_type: GitHubRepoType::All, + github_repo_type: GitHubRepoType::Source, // GitLab gitlab_user: Vec::new(), From 0bdd68c90019f02be5443d957089ccf3983cbf08 Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Thu, 7 Aug 2025 18:30:40 -0700 Subject: [PATCH 5/6] JWT tokens without both 'iss' and 'aud' are no longer reported as active credentials --- src/cli/commands/github.rs | 3 --- src/gitlab.rs | 2 +- src/validation/jwt.rs | 28 +++++++++++++++++++++++- tests/int_redact.rs | 2 +- tests/int_rules_no_validated_findings.rs | 13 +++-------- 5 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/cli/commands/github.rs b/src/cli/commands/github.rs index 50aa190..7537c2e 100644 --- a/src/cli/commands/github.rs +++ b/src/cli/commands/github.rs @@ -75,8 +75,6 @@ impl GitHubRepoSpecifiers { #[derive(Copy, Clone, Debug, Display, PartialEq, Eq, PartialOrd, Ord, ValueEnum)] #[strum(serialize_all = "kebab-case")] pub enum GitHubRepoType { - /// Both source and fork repositories - All, /// Only source repositories (not forks) Source, /// Only fork repositories @@ -87,7 +85,6 @@ pub enum GitHubRepoType { impl From for crate::github::RepoType { fn from(val: GitHubRepoType) -> Self { match val { - GitHubRepoType::Source => crate::github::RepoType::All, GitHubRepoType::Source => crate::github::RepoType::Source, GitHubRepoType::Fork => crate::github::RepoType::Fork, } diff --git a/src/gitlab.rs b/src/gitlab.rs index d94f46c..c3bd5ea 100644 --- a/src/gitlab.rs +++ b/src/gitlab.rs @@ -104,7 +104,7 @@ pub async fn enumerate_repo_urls( } } - let projects_ep = builder.build()?; // now no borrows of a temporary + let projects_ep = builder.build()?; // now no borrows of a temporary let projects: Vec = projects_ep.query(&client)?; for proj in projects { diff --git a/src/validation/jwt.rs b/src/validation/jwt.rs index d5485d6..c5649a9 100644 --- a/src/validation/jwt.rs +++ b/src/validation/jwt.rs @@ -69,6 +69,26 @@ pub async fn validate_jwt(token: &str) -> Result<(bool, String)> { } } + let header_b64 = token.split('.').next().ok_or_else(|| anyhow!("invalid JWT format"))?; + let header_json = + URL_SAFE_NO_PAD.decode(header_b64).map_err(|e| anyhow!("invalid base64 in header: {e}"))?; + let header_val: serde_json::Value = + serde_json::from_slice(&header_json).map_err(|e| anyhow!("invalid header json: {e}"))?; + let alg_str = header_val.get("alg").and_then(|v| v.as_str()).unwrap_or(""); + + // If alg is "none", skip signature/JWKS entirely + if alg_str.eq_ignore_ascii_case("none") { + // still enforce your time/claims checks that already ran + return Ok(( + true, + format!( + "JWT valid (alg: none, iss: {}, aud: {:?})", + claims.iss.clone().unwrap_or_default(), + extract_aud_strings(&claims), + ), + )); + } + // --------------------------------------------------------------------------- let issuer = claims.iss.clone().unwrap_or_default(); let aud_strings = extract_aud_strings(&claims); @@ -200,7 +220,13 @@ mod tests { fn build_token(exp_offset: i64) -> String { let header = URL_SAFE_NO_PAD.encode(r#"{"alg":"none"}"#); let exp = (Utc::now() + ChronoDuration::seconds(exp_offset)).timestamp(); - let payload = URL_SAFE_NO_PAD.encode(format!("{{\"exp\":{exp}}}")); + let payload = URL_SAFE_NO_PAD.encode(format!( + r#"{{ + "exp": {exp}, + "iss": "https://example.com", + "aud": ["test-audience"] + }}"# + )); format!("{header}.{payload}.") } diff --git a/tests/int_redact.rs b/tests/int_redact.rs index 5d72c61..796d019 100644 --- a/tests/int_redact.rs +++ b/tests/int_redact.rs @@ -112,4 +112,4 @@ async fn test_redact_hashes_finding_values() -> Result<()> { } Ok(()) -} \ No newline at end of file +} diff --git a/tests/int_rules_no_validated_findings.rs b/tests/int_rules_no_validated_findings.rs index 51d4a3b..aea1cb0 100644 --- a/tests/int_rules_no_validated_findings.rs +++ b/tests/int_rules_no_validated_findings.rs @@ -5,12 +5,7 @@ use serde_json::Value; #[test] fn scan_rules_has_no_validated_findings() -> Result<()> { let output = Command::cargo_bin("kingfisher")? - .args([ - "scan", "data/rules", - "--format", "json", - "--no-update-check", - "--only-valid", - ]) + .args(["scan", "data/rules", "--format", "json", "--no-update-check", "--only-valid"]) .output()?; let stdout = String::from_utf8_lossy(&output.stdout); @@ -52,10 +47,8 @@ fn scan_rules_has_no_validated_findings() -> Result<()> { let rule_id = finding["rule"]["id"].as_str().unwrap_or("unknown"); let rule_prevalidated = finding["rule"]["prevalidated"].as_bool().unwrap_or(false); - let status = finding["finding"]["validation"]["status"] - .as_str() - .unwrap_or("") - .to_ascii_lowercase(); + let status = + finding["finding"]["validation"]["status"].as_str().unwrap_or("").to_ascii_lowercase(); let response = finding["finding"]["validation"]["response"] .as_str() From a912043eb9cd1c41d6f13d54c6420890b2ed482a Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Thu, 7 Aug 2025 18:45:46 -0700 Subject: [PATCH 6/6] changes in response to code review --- src/gitlab.rs | 6 ++++-- src/validation/jwt.rs | 2 +- tests/int_rules_no_validated_findings.rs | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/gitlab.rs b/src/gitlab.rs index c3bd5ea..f5b6ee3 100644 --- a/src/gitlab.rs +++ b/src/gitlab.rs @@ -103,8 +103,10 @@ pub async fn enumerate_repo_urls( // nothing } } - - let projects_ep = builder.build()?; // now no borrows of a temporary + + // Extract the builder to a separate variable to avoid borrowing a temporary, + // allowing us to modify its fields before building the endpoint. + let projects_ep = builder.build()?; let projects: Vec = projects_ep.query(&client)?; for proj in projects { diff --git a/src/validation/jwt.rs b/src/validation/jwt.rs index c5649a9..bf4fc5f 100644 --- a/src/validation/jwt.rs +++ b/src/validation/jwt.rs @@ -94,7 +94,7 @@ pub async fn validate_jwt(token: &str) -> Result<(bool, String)> { let aud_strings = extract_aud_strings(&claims); if issuer.trim().is_empty() && aud_strings.iter().all(|s| s.trim().is_empty()) { - return Ok((false, "JWT missing issuer and audience".to_string())); + return Ok((false, "JWT missing issuer and audience".into())); } if let Some(iss) = claims.iss.clone() { // parse header now (kid, alg) diff --git a/tests/int_rules_no_validated_findings.rs b/tests/int_rules_no_validated_findings.rs index aea1cb0..a6d171d 100644 --- a/tests/int_rules_no_validated_findings.rs +++ b/tests/int_rules_no_validated_findings.rs @@ -62,7 +62,7 @@ fn scan_rules_has_no_validated_findings() -> Result<()> { // Fail only on genuinely validated secrets assert_ne!( - status.as_str(), + &status, "active credential", "Validated finding detected in rule {rule_id}" );