diff --git a/CHANGELOG.md b/CHANGELOG.md index 482eca5..d6bc09d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ 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". - Fixed issue where `--redact` did not function properly 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 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/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..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::All => 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 13bc78b..2249640 100644 --- a/src/cli/commands/inputs.rs +++ b/src/cli/commands/inputs.rs @@ -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..f5b6ee3 100644 --- a/src/gitlab.rs +++ b/src/gitlab.rs @@ -88,9 +88,27 @@ 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 + } + } + + // 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 { repo_urls.push(proj.http_url_to_repo); } @@ -102,19 +120,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..58145e6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -281,7 +281,7 @@ fn create_default_scan_args() -> cli::commands::scan::ScanArgs { 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..aae16fc 100644 --- a/src/reporter/json_format.rs +++ b/src/reporter/json_format.rs @@ -83,7 +83,7 @@ mod tests { 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, 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..bf4fc5f 100644 --- a/src/validation/jwt.rs +++ b/src/validation/jwt.rs @@ -69,9 +69,33 @@ 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); + if issuer.trim().is_empty() && aud_strings.iter().all(|s| s.trim().is_empty()) { + return Ok((false, "JWT missing issuer and audience".into())); + } if let Some(iss) = claims.iss.clone() { // parse header now (kid, alg) let header = decode_header(token).map_err(|e| anyhow!("decode header: {e}"))?; @@ -196,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 new file mode 100644 index 0000000..a6d171d --- /dev/null +++ b/tests/int_rules_no_validated_findings.rs @@ -0,0 +1,72 @@ +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, + "active credential", + "Validated finding detected in rule {rule_id}" + ); + } + + Ok(()) +}