diff --git a/README.md b/README.md index 10fbec2..219d80e 100644 --- a/README.md +++ b/README.md @@ -288,10 +288,11 @@ Every key holds its real value, so a Run reading it does exactly what it does wi 2. Every Run a Merge run? (`merge.always`) 3. With that on, every Run may start a Base fix when the Base branch's CI is red? (`base.fix`). With it off, this is not asked, and `base.fix` is written at its default, `false`. 4. Every Run first fast-forwards your checkout of the Base branch? (`launch.pull`) -5. Run notifications? (`email.always`). If yes, the address (`email.to`), asked again until it has an `@`, then the sender (`email.from`). -6. With notifications on, the Resend API key, with input hidden. With none saved it asks `Resend API key (input hidden, Enter to skip):`; with one in the [Credentials](#email), `Resend API key (input hidden, Enter keeps the saved one):`, and a new one replaces it. Surrounding spaces are trimmed, and anything that doesn't start with `re_` is asked again. A key you give is saved in the Credentials, `~/.thirdshift/credentials.toml`, created with mode 600 (and `~/.thirdshift` with it) or edited in place, keeping its comments and anything else in it and changing only `resend.key`; `setup` then prints `wrote the Credentials `. The key is never printed, nor written to the User config. Skipping writes no Credentials and says how to add a key later: rerun `thirdshift setup`, or set `RESEND_API_KEY`. With `RESEND_API_KEY` set and not empty, which wins over the Credentials, nothing is asked, and it says the key comes from `RESEND_API_KEY`. With a key found or given, it offers to send a test email (default No), as `thirdshift email-test` does, once the files are written. +5. Security runs may fix reproduced findings? (`security.fix`), off by default. +6. Run notifications? (`email.always`). If yes, the address (`email.to`), asked again until it has an `@`, then the sender (`email.from`). +7. With notifications on, the Resend API key, with input hidden. With none saved it asks `Resend API key (input hidden, Enter to skip):`; with one in the [Credentials](#email), `Resend API key (input hidden, Enter keeps the saved one):`, and a new one replaces it. Surrounding spaces are trimmed, and anything that doesn't start with `re_` is asked again. A key you give is saved in the Credentials, `~/.thirdshift/credentials.toml`, created with mode 600 (and `~/.thirdshift` with it) or edited in place, keeping its comments and anything else in it and changing only `resend.key`; `setup` then prints `wrote the Credentials `. The key is never printed, nor written to the User config. Skipping writes no Credentials and says how to add a key later: rerun `thirdshift setup`, or set `RESEND_API_KEY`. With `RESEND_API_KEY` set and not empty, which wins over the Credentials, nothing is asked, and it says the key comes from `RESEND_API_KEY`. With a key found or given, it offers to send a test email (default No), as `thirdshift email-test` does, once the files are written. -Pressing Enter takes the default shown, which is the file's current value, or else the setting's default, and for the address the suggested email above. `logs.dir`, `activity.quiet_skips`, `spec.parallel` and `pickup.limit` are not asked about. The answers are written like everything else below: in place, keeping your comments. Ctrl-C, SIGTERM or SIGHUP during the questions or a Model/Effort check exits `1` with `interrupted` and writes nothing: neither the User config nor the Credentials. Ordinary and hidden questions stop without another line of input, and hidden input restores the saved terminal settings, including echo. With notifications off, nothing about a key is asked, and saved Credentials stay as they were, so `--email` on a single Run still works. Credentials a Run would refuse (see [Email](#email)) are refused before any question, exit `1`, and not touched. With no terminal, as from cron or `thirdshift setup . Set your defaults now? [Y/n]`. Yes (or Enter) asks the questions above, writes the file, and the Run carries on using your answers; a flag in the command, such as `--no-merge`, `--email` or `--no-email`, still wins over them. No writes every setting at its default, as `setup` with no terminal does, asks nothing about a key, writes no Credentials, says that `thirdshift setup` changes it, and the Run carries on; later Runs find the file and don't offer again. If the file can't be written, stderr gets a `warning:` line and the Run carries on with the defaults. Ctrl-C, SIGTERM or SIGHUP during the offer, the questions or a Model/Effort check exits `1` with `interrupted`, writes nothing and ends the command before any work, with no Run notification. A command thirdshift can't parse exits `2` before any offer. A Run with no terminal, as from cron, CI, `nohup` or an agent's shell, offers nothing, writes nothing, and runs on the defaults, so a later Run from a terminal still gets the offer. diff --git a/src/config.rs b/src/config.rs index cbd51aa..88f22d7 100644 --- a/src/config.rs +++ b/src/config.rs @@ -212,6 +212,7 @@ pub struct UserConfigChanges { pub merge_always: bool, pub base_fix: bool, pub launch_pull: bool, + pub security_fix: bool, /// None disables Run notifications while retaining their addresses. pub notifications: Option, /// None retains every Harness setting. Unset Model/Effort writes blank. @@ -260,6 +261,7 @@ impl UserConfigDocument { set(&mut document, "merge", "always", changes.merge_always); set(&mut document, "base", "fix", changes.base_fix); set(&mut document, "launch", "pull", changes.launch_pull); + set(&mut document, "security", "fix", changes.security_fix); set( &mut document, "email", @@ -1185,6 +1187,7 @@ mod tests { merge_always: true, base_fix: true, launch_pull: true, + security_fix: false, notifications: Some(NotificationAddresses { to: "me@example.com".to_string(), from: "ts@example.com".to_string(), @@ -1338,6 +1341,7 @@ mod tests { merge_always: false, base_fix: false, launch_pull: false, + security_fix: false, notifications: Some(NotificationAddresses { to: to.to_string(), from: crate::email::DEFAULT_FROM.to_string(), @@ -1430,6 +1434,7 @@ mod tests { merge_always: true, base_fix: false, launch_pull: false, + security_fix: false, notifications: None, harness: Some(( Harness::Codex, @@ -1473,6 +1478,7 @@ mod tests { merge_always: true, base_fix: true, launch_pull: true, + security_fix: false, notifications: Some(NotificationAddresses { to: "o\"brien@example.com".to_string(), from: "ts@example.com".to_string(), @@ -1557,6 +1563,7 @@ mod tests { merge_always: true, base_fix: false, launch_pull: true, + security_fix: false, notifications: None, harness: None, }; diff --git a/src/setup.rs b/src/setup.rs index dbb9310..0407f1f 100644 --- a/src/setup.rs +++ b/src/setup.rs @@ -719,6 +719,7 @@ mod tests { const MERGE: &str = "Every Run a Merge run?"; const BASE_FIX: &str = "Every Run may start a Base fix when the Base branch's CI is red?"; const PULL: &str = "fast-forward"; + const SECURITY_FIX: &str = "Security runs may fix reproduced findings?"; const NOTIFY: &str = "Run notifications, an email"; const TO: &str = "Send Run notifications to"; const FROM: &str = "Send them from"; @@ -733,7 +734,8 @@ mod tests { const KEY: &str = "re_secret_123"; /// The answers that take every default and turn nothing on. - const ENTER_THROUGHOUT: [(&str, &str); 3] = [(MERGE, ""), (PULL, ""), (NOTIFY, "")]; + const ENTER_THROUGHOUT: [(&str, &str); 4] = + [(MERGE, ""), (PULL, ""), (SECURITY_FIX, ""), (NOTIFY, "")]; /// The answers that turn Run notifications on, to `me@example.com` from /// the default sender, then `rest`. @@ -743,6 +745,7 @@ mod tests { let mut answers = vec![ (MERGE, ""), (PULL, ""), + (SECURITY_FIX, ""), (NOTIFY, "y"), (TO, "me@example.com"), (FROM, ""), @@ -1044,6 +1047,7 @@ to = \"me@example.com\" # my inbox [ "Every Run a Merge run? [y/N] ", "Every Run first fast-forwards your checkout of the Base branch? [y/N] ", + "Security runs may fix reproduced findings? [y/N] ", "Run notifications, an email as each Run ends? [y/N] ", ] ); @@ -1057,6 +1061,7 @@ to = \"me@example.com\" # my inbox (MERGE, "y"), (BASE_FIX, "y"), (PULL, "yes"), + (SECURITY_FIX, ""), (NOTIFY, "y"), (TO, "me@example.com"), (FROM, "ts@acme.dev"), @@ -1164,6 +1169,7 @@ effort = \"\" (MERGE, ""), (BASE_FIX, ""), (PULL, ""), + (SECURITY_FIX, ""), (NOTIFY, ""), (TO, ""), (FROM, ""), @@ -1184,6 +1190,7 @@ effort = \"\" "Every Run a Merge run? [Y/n] ", "Every Run may start a Base fix when the Base branch's CI is red? [Y/n] ", "Every Run first fast-forwards your checkout of the Base branch? [Y/n] ", + "Security runs may fix reproduced findings? [y/N] ", "Run notifications, an email as each Run ends? [Y/n] ", "Send Run notifications to [mine@example.net]: ", "Send them from [ts@acme.dev]: ", @@ -1196,7 +1203,7 @@ effort = \"\" fn on_a_terminal_changed_answers_are_written_over_the_user_config() { let mut outside = Scripted { user_config: Some(DEFAULTS.to_string()), - ..Scripted::answering(&[(MERGE, ""), (PULL, "y"), (NOTIFY, "")]) + ..Scripted::answering(&[(MERGE, ""), (PULL, "y"), (SECURITY_FIX, ""), (NOTIFY, "")]) }; let done = setup(&mut outside).unwrap(); @@ -1227,6 +1234,7 @@ always = false # quiet, please (MERGE, "y"), (BASE_FIX, ""), (PULL, ""), + (SECURITY_FIX, ""), (NOTIFY, "y"), (TO, "me@example.com"), (FROM, ""), @@ -1328,8 +1336,13 @@ always = false # quiet, please #[test] fn with_merging_on_setup_asks_about_base_fixes_and_writes_the_answer() { for (answer, allowed) in [("y", true), ("", false), ("n", false)] { - let mut outside = - Scripted::answering(&[(MERGE, "y"), (BASE_FIX, answer), (PULL, ""), (NOTIFY, "")]); + let mut outside = Scripted::answering(&[ + (MERGE, "y"), + (BASE_FIX, answer), + (PULL, ""), + (SECURITY_FIX, ""), + (NOTIFY, ""), + ]); setup(&mut outside).unwrap(); @@ -1349,7 +1362,12 @@ always = false # quiet, please #[test] fn with_merging_off_setup_asks_nothing_about_base_fixes_and_writes_the_default() { for answer in ["", "n"] { - let mut outside = Scripted::answering(&[(MERGE, answer), (PULL, ""), (NOTIFY, "")]); + let mut outside = Scripted::answering(&[ + (MERGE, answer), + (PULL, ""), + (SECURITY_FIX, ""), + (NOTIFY, ""), + ]); setup(&mut outside).unwrap(); @@ -1363,7 +1381,13 @@ always = false # quiet, please fn the_base_fix_question_defaults_to_the_user_configs_base_fix() { let mut outside = Scripted { user_config: Some("[merge]\nalways = true\n\n[base]\nfix = true # mine\n".to_string()), - ..Scripted::answering(&[(MERGE, ""), (BASE_FIX, ""), (PULL, ""), (NOTIFY, "")]) + ..Scripted::answering(&[ + (MERGE, ""), + (BASE_FIX, ""), + (PULL, ""), + (SECURITY_FIX, ""), + (NOTIFY, ""), + ]) }; setup(&mut outside).unwrap(); @@ -1383,7 +1407,7 @@ always = false # quiet, please fn turning_merging_off_writes_base_fix_at_its_default() { let mut outside = Scripted { user_config: Some("[merge]\nalways = true\n\n[base]\nfix = true # mine\n".to_string()), - ..Scripted::answering(&[(MERGE, "n"), (PULL, ""), (NOTIFY, "")]) + ..Scripted::answering(&[(MERGE, "n"), (PULL, ""), (SECURITY_FIX, ""), (NOTIFY, "")]) }; setup(&mut outside).unwrap(); @@ -1400,6 +1424,7 @@ always = false # quiet, please (MERGE, "YES"), (BASE_FIX, "No"), (PULL, ""), + (SECURITY_FIX, ""), (NOTIFY, ""), ]); @@ -1413,6 +1438,7 @@ always = false # quiet, please let mut outside = Scripted::answering(&[ (MERGE, ""), (PULL, ""), + (SECURITY_FIX, ""), (NOTIFY, "y"), (TO, ""), (TO, "me.example.com"), @@ -1441,7 +1467,7 @@ always = false # quiet, please let mut outside = Scripted { key: Ok(Some(Source::Credentials(CREDENTIALS.into()))), github_email: Ok(Some("octo@example.com")), - ..Scripted::answering(&[(MERGE, ""), (PULL, ""), (NOTIFY, "n")]) + ..Scripted::answering(&[(MERGE, ""), (PULL, ""), (SECURITY_FIX, ""), (NOTIFY, "n")]) }; setup(&mut outside).unwrap(); @@ -1683,6 +1709,7 @@ always = false # quiet, please ..Scripted::answering(&[ (MERGE, ""), (PULL, ""), + (SECURITY_FIX, ""), (NOTIFY, "y"), (TO, ""), (FROM, ""), @@ -1783,6 +1810,7 @@ always = false # quiet, please (MERGE, "y"), (BASE_FIX, ""), (PULL, ""), + (SECURITY_FIX, ""), (NOTIFY, ""), ]); @@ -1801,6 +1829,7 @@ always = false # quiet, please (OFFER, "y"), (MERGE, ""), (PULL, ""), + (SECURITY_FIX, ""), (NOTIFY, "y"), (TO, "me@example.com"), (FROM, ""), @@ -1898,6 +1927,7 @@ always = false # quiet, please (OFFER, "y"), (MERGE, ""), (PULL, ""), + (SECURITY_FIX, ""), (NOTIFY, "y"), (TO, "me@example.com"), (FROM, ""), diff --git a/src/setup/questions.rs b/src/setup/questions.rs index 33398e4..f9a90be 100644 --- a/src/setup/questions.rs +++ b/src/setup/questions.rs @@ -26,6 +26,8 @@ pub struct Answers { pub base_fix: bool, /// `launch.pull`. pub launch_pull: bool, + /// `security.fix`: Security runs may fix reproduced findings. + pub security_fix: bool, /// With Run notifications on, `email.always`, their settings; with them /// off, `None`, and the email settings stay as they were. pub notifications: Option, @@ -38,6 +40,7 @@ impl Answers { merge_always: self.merge_always, base_fix: self.base_fix, launch_pull: self.launch_pull, + security_fix: self.security_fix, notifications: self .notifications .as_ref() @@ -118,6 +121,11 @@ pub fn ask( "Every Run first fast-forwards your checkout of the Base branch?", current.launch_pull, )?; + let security_fix = yes_or_no( + outside, + "Security runs may fix reproduced findings?", + current.security_fix.unwrap_or(false), + )?; if !yes_or_no( outside, "Run notifications, an email as each Run ends?", @@ -128,6 +136,7 @@ pub fn ask( merge_always, base_fix, launch_pull, + security_fix, notifications: None, }); } @@ -169,6 +178,7 @@ pub fn ask( merge_always, base_fix, launch_pull, + security_fix, notifications: Some(Notifications { to, from, diff --git a/tests/muse_sessions.rs b/tests/muse_sessions.rs index a4e74f1..4a77d71 100644 --- a/tests/muse_sessions.rs +++ b/tests/muse_sessions.rs @@ -297,6 +297,7 @@ fn setup_checks_muses_proposed_model_with_the_same_flags_and_environment() { ("Effort for muse", "low"), ("Every Run a Merge run", ""), ("Every Run first fast-forwards", ""), + ("Security runs may fix reproduced findings?", ""), ("Run notifications", ""), ], ); diff --git a/tests/opencode_sessions.rs b/tests/opencode_sessions.rs index 5a9922a..c14fb09 100644 --- a/tests/opencode_sessions.rs +++ b/tests/opencode_sessions.rs @@ -406,6 +406,7 @@ fn setup_proposes_no_model_and_checks_the_answer_standalone() { ("Effort for opencode", "high"), ("Every Run a Merge run", ""), ("Every Run first fast-forwards", ""), + ("Security runs may fix reproduced findings?", ""), ("Run notifications", ""), ], ); @@ -482,6 +483,7 @@ fn setup_retries_a_refused_model_and_effort_with_the_same_preflight_check() { ("Effort for opencode", "low"), ("Every Run a Merge run", ""), ("Every Run first fast-forwards", ""), + ("Security runs may fix reproduced findings?", ""), ("Run notifications", ""), ], ); diff --git a/tests/setup.rs b/tests/setup.rs index 7baddb8..d598b8a 100644 --- a/tests/setup.rs +++ b/tests/setup.rs @@ -75,6 +75,7 @@ fn with_no_terminal_and_no_user_config_setup_writes_the_defaults() { assert_eq!(config["activity"]["quiet_skips"].as_bool(), Some(false)); assert_eq!(config["spec"]["parallel"].as_integer(), Some(3)); assert_eq!(config["pickup"]["limit"].as_integer(), Some(3)); + assert_eq!(config["security"]["fix"].as_bool(), Some(false)); assert_eq!(config["security"]["harness"].as_str(), Some("")); assert_eq!(config["harness"]["default"].as_str(), Some("claude")); for harness in ["claude", "codex", "agy", "grok", "muse", "opencode"] { @@ -187,6 +188,7 @@ const EFFORT: &str = "Effort for claude"; const MERGE: &str = "Merge run?"; const BASE_FIX: &str = "Every Run may start a Base fix when the Base branch's CI is red?"; const PULL: &str = "fast-forward"; +const SECURITY_FIX: &str = "Security runs may fix reproduced findings?"; const NOTIFY: &str = "Run notifications, an email"; const TO: &str = "Send Run notifications to"; const FROM: &str = "Send them from"; @@ -196,6 +198,67 @@ const KEPT: &str = "Resend API key (input hidden, Enter keeps the saved one):"; const WROTE_CREDENTIALS: &str = "wrote the Credentials"; const KEY: &str = "re_secret_123"; +#[test] +fn setup_asks_once_about_security_fixing_with_off_as_the_default_and_writes_yes() { + let scenario = Scenario::new(); + let result = setup_on_terminal( + &scenario, + &[], + &[ + (HARNESS, ""), + (MODEL, ""), + (EFFORT, ""), + (MERGE, ""), + (PULL, ""), + (SECURITY_FIX, "y"), + (NOTIFY, ""), + ], + ); + assert_eq!(result.stderr.matches(SECURITY_FIX).count(), 1); + assert!(result.stderr.contains(&format!("{SECURITY_FIX} [y/N]"))); + let text = result.user_config.unwrap(); + let config: toml::Table = text.parse().unwrap(); + assert_eq!(config["security"]["fix"].as_bool(), Some(true)); + assert_eq!(config["security"]["harness"].as_str(), Some("")); + assert!(text.contains("# the Harness a Security run uses unless its command names one")); +} + +#[test] +fn rerunning_setup_keeps_security_answers_as_defaults_and_can_turn_fixing_off() { + let scenario = Scenario::new(); + let saved = "# my Security settings\n[security]\nfix = true # keep this choice\nharness = 'codex' # use Codex\n"; + scenario.user_config_is(saved); + + for (answer, enabled, choices) in [ + ("", true, "[Y/n]"), + ("n", false, "[Y/n]"), + ("", false, "[y/N]"), + ] { + let result = setup_on_terminal( + &scenario, + &[], + &[ + (HARNESS, ""), + (MODEL, ""), + (EFFORT, ""), + (MERGE, ""), + (PULL, ""), + (SECURITY_FIX, answer), + (NOTIFY, ""), + ], + ); + assert!(result.stderr.contains(&format!("{SECURITY_FIX} {choices}"))); + assert_eq!(result.stderr.matches(SECURITY_FIX).count(), 1); + let text = result.user_config.unwrap(); + let config: toml::Table = text.parse().unwrap(); + assert_eq!(config["security"]["fix"].as_bool(), Some(enabled)); + assert_eq!(config["security"]["harness"].as_str(), Some("codex")); + assert!(text.starts_with("# my Security settings\n[security]\n")); + assert!(text.contains("# keep this choice\n")); + assert!(text.contains("harness = 'codex' # use Codex\n")); + } +} + #[test] fn cancelling_a_claude_setup_check_cleans_up_without_writing_or_retrying() { use std::time::{Duration, Instant}; @@ -343,6 +406,7 @@ fn cancelling_later_questions_after_successful_or_repeated_checks_restores_echo_ keys.extend([ TerminalStep::line(MERGE, ""), TerminalStep::line(PULL, ""), + TerminalStep::line(SECURITY_FIX, ""), TerminalStep::line(NOTIFY, "y"), TerminalStep::line(TO, "me@example.com"), TerminalStep::line(FROM, ""), @@ -384,7 +448,7 @@ fn pasted_answers_are_left_available_to_later_terminal_questions() { let result = setup_on_terminal( &scenario, &[], - &[(HARNESS, " claude \n\n\n\n\ny\n café@example.com \n\n")], + &[(HARNESS, " claude \n\n\n\n\n\ny\n café@example.com \n\n")], ); let config: toml::Table = result.user_config.unwrap().parse().unwrap(); assert_eq!(config["harness"]["default"].as_str(), Some("claude")); @@ -425,6 +489,7 @@ fn a_partial_final_terminal_answer_is_trimmed_and_accepted_before_eof() { TerminalStep::line(EFFORT, ""), TerminalStep::line(MERGE, ""), TerminalStep::line(PULL, ""), + TerminalStep::line(SECURITY_FIX, ""), // Queue EOF for both the partial answer and the next question. // macOS can finish both reads before another input action runs. TerminalStep::bytes(NOTIFY, b" y \x04\x04\x04"), @@ -464,6 +529,7 @@ fn notifications_on<'a>(rest: &[Keystrokes<'a>]) -> Vec> { (EFFORT, ""), (MERGE, ""), (PULL, ""), + (SECURITY_FIX, ""), (NOTIFY, "y"), (TO, "me@example.com"), (FROM, ""), @@ -489,18 +555,23 @@ fn on_a_terminal_pressing_enter_throughout_writes_what_setup_with_no_terminal_wr (EFFORT, ""), (MERGE, ""), (PULL, ""), + (SECURITY_FIX, ""), (NOTIFY, ""), ], ); assert_eq!(result.user_config, user_config(&unattended)); - for prompt in [MERGE, PULL, NOTIFY] { + for prompt in [MERGE, PULL, SECURITY_FIX, NOTIFY] { assert!( result.stderr.contains(prompt), "terminal: {}", result.stderr ); } + assert_eq!(result.stderr.matches(SECURITY_FIX).count(), 1); + assert!(result.stderr.contains(&format!("{SECURITY_FIX} [y/N]"))); + let config: toml::Table = result.user_config.unwrap().parse().unwrap(); + assert_eq!(config["security"]["fix"].as_bool(), Some(false)); } #[test] @@ -573,7 +644,8 @@ fn ctrl_c_during_the_questions_writes_no_user_config() { (EFFORT, ""), (MERGE, "y"), (BASE_FIX, ""), - (PULL, CTRL_C), + (PULL, ""), + (SECURITY_FIX, CTRL_C), ], ); @@ -598,6 +670,7 @@ fn ctrl_c_during_the_questions_leaves_an_existing_user_config_unchanged() { (EFFORT, ""), (MERGE, "n"), (PULL, "y"), + (SECURITY_FIX, ""), (NOTIFY, "y"), (TO, CTRL_C), ], diff --git a/tests/setup_offer.rs b/tests/setup_offer.rs index 2974a96..bf6427f 100644 --- a/tests/setup_offer.rs +++ b/tests/setup_offer.rs @@ -28,6 +28,7 @@ const EFFORT: &str = "Effort for claude"; const MERGE: &str = "Merge run?"; const BASE_FIX: &str = "Every Run may start a Base fix when the Base branch's CI is red?"; const PULL: &str = "fast-forward"; +const SECURITY_FIX: &str = "Security runs may fix reproduced findings?"; const NOTIFY: &str = "Run notifications, an email"; const KEY: &str = "re_secret_123"; @@ -82,6 +83,7 @@ fn accepting_and_choosing_merge_always_makes_that_run_a_merge_run() { (MERGE, "y"), (BASE_FIX, ""), (PULL, ""), + (SECURITY_FIX, ""), (NOTIFY, ""), ], );