Pick any line to see why it is the way it is: the commit, the pull request and issue it came from, and what the agent was thinking.
| Teams and CODEOWNERS, labels and milestones, dependency updates, the security suite, and a clearer top bar | 1 | //! "Fix with g1t": an alert becomes an issue assigned to g1t, which writes |
| 2 | //! the fix in a pull request that lands through the repository's required | |
| 3 | //! checks, as any agent's work does. Nothing new is built for it: it is the | |
| 4 | //! issue → agent → checks → queue pipeline, and the agent's run is charged | |
| 5 | //! as agent usage. | |
| 6 | ||
| 7 | use g1t_contracts::repos::RepoPath; | |
| 8 | use g1t_contracts::security::{SecretFinding, Vulnerability}; | |
| 9 | use g1t_contracts::security_suite::{AlertFix, AlertType, CodeAlert, FixAlertArgs, PaidFeature}; | |
| 10 | use g1t_contracts::work::{Issue, OpenIssueArgs, with_definition_of_done}; | |
| 11 | use g1t_contracts::{FailureCode, Outcome}; | |
| 12 | use serde_json::{Value, json}; | |
| 13 | use worker::Result; | |
| 14 | ||
| 15 | use crate::Security; | |
| 16 | use crate::store::{Activity, VulnRow}; | |
| 17 | ||
| 18 | fn fail<T>(code: FailureCode, message: impl Into<String>) -> Outcome<T> { | |
| 19 | Outcome::fail(code, message) | |
| 20 | } | |
| 21 | ||
| 22 | const FOOTER: &str = "\n\n---\n_Opened from a security alert with Fix with g1t. The pull request lands through this repository's required checks._"; | |
| 23 | ||
| 24 | /// The issue for a code scanning alert. | |
| 25 | pub fn code_issue(alert: &CodeAlert, page: &str) -> (String, String) { | |
| 26 | let place = match (&alert.path, alert.start_line) { | |
| 27 | (Some(path), Some(line)) => format!("`{path}` line {line}"), | |
| 28 | (Some(path), None) => format!("`{path}`"), | |
| 29 | _ => "the repository".to_owned(), | |
| 30 | }; | |
| 31 | let rule = alert.rule_name.as_deref().unwrap_or(&alert.rule_id); | |
| 32 | let title = format!("Fix code scanning alert #{}: {rule}", alert.number); | |
| 33 | let mut body = format!( | |
| 34 | "{} reports **{}** ({}) in {place}:\n\n> {}\n\n", | |
| 35 | alert.tool, | |
| 36 | alert.rule_id, | |
| 37 | alert.security_severity.as_deref().unwrap_or(&alert.level), | |
| 38 | alert.message.replace('\n', "\n> ") | |
| 39 | ); | |
| 40 | if let Some(description) = &alert.rule_description { | |
| 41 | body.push_str(&format!("{description}\n\n")); | |
| 42 | } | |
| 43 | if let Some(help) = &alert.help { | |
| 44 | body.push_str(&format!("<details><summary>About this rule</summary>\n\n{help}\n\n</details>\n\n")); | |
| 45 | } | |
| 46 | body.push_str(&format!( | |
| 47 | "Change the code so the problem is gone, not hidden: no suppression comments, and no change to the analysis. \ | |
| 48 | Keep the change to what the fix needs. The alert: {page}\n" | |
| 49 | )); | |
| 50 | let done = vec![ | |
| 51 | format!("{} no longer reports `{}` at {place} when it runs on the pull request.", alert.tool, alert.rule_id), | |
| 52 | "The project's tests still pass.".to_owned(), | |
| 53 | ]; | |
| 54 | (title, format!("{}{FOOTER}", with_definition_of_done(&body, &done))) | |
| 55 | } | |
| 56 | ||
| 57 | /// The issue for a leaked secret: take it out of the code. Rotating it is | |
| 58 | /// the person's: only they can at its issuer. | |
| 59 | pub fn secret_issue(secret: &SecretFinding, page: &str) -> (String, String) { | |
| 60 | let title = format!("Remove {} from {}", secret.label, secret.path); | |
| 61 | let body = format!( | |
| 62 | "{} (`{}`) is in `{}` at line {}, committed in {}. Take it out of the code and read it from configuration \ | |
| 63 | instead: an environment variable, or the repository's Actions secrets for workflows. Do not write the value \ | |
| 64 | anywhere else, including in tests, docs or this pull request.\n\nRemoving it from the code does not make it \ | |
| 65 | safe: it is in the history. Whoever owns it has to rotate it at its issuer, then mark the alert revoked. \ | |
| 66 | The alert: {page}\n", | |
| 67 | secret.label, | |
| 68 | secret.preview, | |
| 69 | secret.path, | |
| 70 | secret.line, | |
| 71 | &secret.commit[..secret.commit.len().min(7)], | |
| 72 | ); | |
| 73 | let done = vec![format!("`{}` no longer holds the secret, and nothing else does instead.", secret.path), "The project's tests still pass.".to_owned()]; | |
| 74 | (title, format!("{}{FOOTER}", with_definition_of_done(&body, &done))) | |
| 75 | } | |
| 76 | ||
| 77 | impl Security { | |
| 78 | pub(crate) async fn fix_alert(&self, a: FixAlertArgs) -> Result<Outcome<AlertFix>> { | |
| 79 | let repo = match self.member_repo(&a.repo, &Some(a.actor.clone()), crate::SEE_FINDINGS).await? { | |
| 80 | Outcome::Ok(repo) => repo, | |
| 81 | Outcome::Fail(failure) => return Ok(Outcome::Fail(failure)), | |
| 82 | }; | |
| 83 | if !a.actor.verified { | |
| 84 | return Ok(fail(FailureCode::Forbidden, "Confirm your email address first.")); | |
| 85 | } | |
| 86 | let page = |rest: &str| format!("https://g1t.sh{}", crate::suite::link(&repo, rest)); | |
| 87 | let path = RepoPath { namespace: repo.namespace.clone(), name: repo.name.clone() }; | |
| 88 | let (title, body, labels, existing) = match AlertType::of_id(&a.id) { | |
| 89 | Some(AlertType::CodeScanning) => { | |
| 90 | if let Some(refusal) = self.gate(&repo, PaidFeature::CodeScanning).await? { | |
| 91 | return Ok(refusal); | |
| 92 | } | |
| 93 | let Some(alert) = self.store.code_alert_by_id(&repo.repo_id, &a.id).await? else { | |
| 94 | return Ok(fail(FailureCode::NotFound, "No such alert.")); | |
| 95 | }; | |
| 96 | let (title, body) = code_issue(&alert, &page(&format!("code-scanning/{}", alert.number))); | |
| 97 | (title, body, vec!["security".to_owned(), "code-scanning".to_owned()], alert.issue) | |
| 98 | } | |
| 99 | Some(AlertType::SecretScanning) => { | |
| 100 | let Some(secret) = self.store.secret(&repo.repo_id, &a.id).await? else { | |
| 101 | return Ok(fail(FailureCode::NotFound, "No such alert.")); | |
| 102 | }; | |
| 103 | if secret.status == g1t_contracts::security::SecretStatus::Blocked { | |
| 104 | return Ok(fail(FailureCode::Conflict, "This secret never landed: take it out of your commit and push again.")); | |
| 105 | } | |
| 106 | let (title, body) = secret_issue(&secret, &page(&format!("secret-scanning/{}", secret.id))); | |
| 107 | (title, body, vec!["security".to_owned()], self.store.fix_issue(&a.id).await?) | |
| 108 | } | |
| 109 | Some(AlertType::Vulnerability) => { | |
| 110 | let Some(vuln) = self.store.vulnerability(&repo.repo_id, &a.id).await? else { | |
| 111 | return Ok(fail(FailureCode::NotFound, "No such alert.")); | |
| 112 | }; | |
| 113 | let (title, body) = self.vulnerability_issue(&repo.repo_id, &vuln).await?; | |
| 114 | (title, body, vec!["dependencies".to_owned(), "security".to_owned()], self.store.fix_issue(&a.id).await?.or(vuln.issue)) | |
| 115 | } | |
| 116 | None => return Ok(fail(FailureCode::NotFound, "No such alert.")), | |
| 117 | }; | |
| 118 | if let Some(number) = existing | |
| 119 | && self.issue(&a.actor, &path, number).await?.is_some_and(|issue| issue.state == g1t_contracts::work::State::Open) | |
| 120 | { | |
| 121 | return Ok(Outcome::Ok(AlertFix { issue: number, started: false, message: Some(format!("g1t is already on it in #{number}.")) })); | |
| 122 | } | |
| 123 | let opened: Outcome<Issue> = g1t_kit::call( | |
| 124 | &self.work, | |
| 125 | "open_issue", | |
| 126 | &OpenIssueArgs { actor: a.actor.clone(), repo: path.clone(), title: title.chars().take(200).collect(), body, labels, checks: Vec::new(), milestone: None }, | |
| 127 | ) | |
| 128 | .await?; | |
| 129 | let issue = match opened { | |
| 130 | Outcome::Ok(issue) => issue, | |
| 131 | Outcome::Fail(failure) => return Ok(Outcome::Fail(failure)), | |
| 132 | }; | |
| 133 | match AlertType::of_id(&a.id) { | |
| 134 | Some(AlertType::CodeScanning) => self.store.set_code_issue(&repo.repo_id, &a.id, issue.number).await?, | |
| 135 | _ => self.store.add_fix(&repo.repo_id, &a.id, issue.number, &a.actor.username).await?, | |
| 136 | } | |
| 137 | self.store | |
| 138 | .record(&repo.repo_id, &[Activity { | |
| 139 | alert_id: &a.id, | |
| 140 | action: "fix_requested", | |
| 141 | actor: Some(&a.actor.username), | |
| 142 | reason: None, | |
| 143 | comment: None, | |
| 144 | number: Some(issue.number), | |
| 145 | }]) | |
| 146 | .await?; | |
| 147 | // g1t takes it, as the person who asked: their agent usage. | |
| 148 | let started: Outcome<Value> = g1t_kit::call(&self.runner, "run", &json!({ "actor": a.actor, "repo": path, "issue": issue.number })).await?; | |
| 149 | Ok(Outcome::Ok(match started { | |
| 150 | Outcome::Ok(_) => AlertFix { issue: issue.number, started: true, message: None }, | |
| 151 | Outcome::Fail(refused) => AlertFix { | |
| 152 | issue: issue.number, | |
| 153 | started: false, | |
| 154 | message: Some(format!("The issue is open, but g1t could not start on it: {}", refused.message)), | |
| 155 | }, | |
| 156 | })) | |
| 157 | } | |
| 158 | ||
| 159 | async fn vulnerability_issue(&self, repo_id: &str, vuln: &Vulnerability) -> Result<(String, String)> { | |
| 160 | let open = self.store.open_vulnerabilities(repo_id).await?; | |
| 161 | let rows: Vec<&VulnRow> = open.iter().filter(|row| row.ecosystem == vuln.ecosystem && row.package == vuln.package).collect(); | |
| 162 | let target = vuln.fixed_version.clone().unwrap_or_else(|| "a version without these advisories".to_owned()); | |
| Security and version updates open no issues on their own: an update whose version cannot be raised fails with why on its alert and is tried again a day later, and one whose required checks fail stays open for a person to decide on; an agent takes one only when a person asks with Fix with g1t. That issue now names the lockfiles the package is locked in and the commands that show it is done, which were left empty ("Locked in: .") because the lockfiles were never passed in. | 163 | let body = crate::deps::issue_text(&vuln.ecosystem, &vuln.package, &target, &rows); |
| Teams and CODEOWNERS, labels and milestones, dependency updates, the security suite, and a clearer top bar | 164 | Ok((format!("Upgrade {} to {target}", vuln.package), body)) |
| 165 | } | |
| 166 | } | |
| 167 | ||
| 168 | #[cfg(test)] | |
| 169 | mod tests { | |
| 170 | use super::*; | |
| 171 | use g1t_contracts::security::AlertState; | |
| 172 | ||
| 173 | #[test] | |
| 174 | fn a_code_alert_becomes_an_issue_with_a_definition_of_done() { | |
| 175 | let alert = CodeAlert { | |
| 176 | id: "cod_1".into(), | |
| 177 | number: 4, | |
| 178 | repo_id: "rep_1".into(), | |
| 179 | tool: "Semgrep OSS".into(), | |
| 180 | category: "Semgrep OSS".into(), | |
| 181 | rule_id: "javascript.browser.security.eval-detected.eval-detected".into(), | |
| 182 | rule_name: None, | |
| 183 | rule_description: Some("Detected the use of eval().".into()), | |
| 184 | help: None, | |
| 185 | help_uri: None, | |
| 186 | tags: Vec::new(), | |
| 187 | level: "warning".into(), | |
| 188 | security_severity: None, | |
| 189 | severity: "medium".into(), | |
| 190 | message: "Detected the use of eval().".into(), | |
| 191 | path: Some("src/server.js".into()), | |
| 192 | start_line: Some(10), | |
| 193 | end_line: Some(10), | |
| 194 | start_column: None, | |
| 195 | end_column: None, | |
| 196 | state: AlertState::Open, | |
| 197 | fingerprint: "f".into(), | |
| 198 | first_commit: "c".into(), | |
| 199 | last_commit: "c".into(), | |
| 200 | created_at: String::new(), | |
| 201 | updated_at: String::new(), | |
| 202 | fixed_at: None, | |
| 203 | dismissed_by: None, | |
| 204 | dismissed_reason: None, | |
| 205 | dismissed_comment: None, | |
| 206 | dismissed_at: None, | |
| 207 | issue: None, | |
| 208 | }; | |
| 209 | let (title, body) = code_issue(&alert, "https://g1t.sh/acme/rocket/security/code-scanning/4"); | |
| 210 | assert_eq!(title, "Fix code scanning alert #4: javascript.browser.security.eval-detected.eval-detected"); | |
| 211 | assert!(body.contains("in `src/server.js` line 10")); | |
| 212 | assert!(body.contains("## Definition of done")); | |
| 213 | assert!(body.contains("no longer reports")); | |
| 214 | assert!(body.contains("no suppression comments")); | |
| 215 | } | |
| 216 | } |
