Skip to content

[Bug]: cron enable and cron disable destroy all existing crontab entries #196

Description

@dongnengyu

Summary

mihoro cron enable and mihoro cron disable both silently destroy the entire user crontab, wiping out any unrelated cron jobs (e.g. acme.sh certificate renewal, backup scripts, panel software, monitoring).

Reproduced on mihoro 0.14.0 on Debian 12.

Reproduction

# starting state — pretend the user has acme.sh cron set up
$ crontab -l
27 16 * * * "/root/.acme.sh"/acme.sh --cron --home "/root/.acme.sh" > /dev/null
0 3 * * * /opt/backup/run.sh

$ mihoro cron enable
mihoro: Auto-update enabled with interval: 12 hours
-> Cron entry: 0 */12 * * * /root/.local/bin/mihoro update

$ crontab -l
0 */12 * * * /root/.local/bin/mihoro update
# acme.sh and backup cron entries are GONE — silently

Root cause

src/cron.rs

enable_auto_update

let crontab_content = generate_crontab(interval_hours)?;  // contains ONLY the mihoro line
fs::write(&crontab_file, crontab_content)?;
let status = std::process::Command::new(\"crontab\")
    .arg(&crontab_file)
    .status()?;

crontab <file> replaces the entire user crontab with the contents of <file>. Since generate_crontab() only emits the mihoro entry, all other entries are lost.

disable_auto_update

let status = std::process::Command::new(\"crontab\").arg(\"-r\").status();

crontab -r removes the entire crontab, not just the mihoro entry. So even users who never had any other cron entry but ran enable then disable end up with no crontab at all (which is harmless), but users who had pre-existing entries from before installing mihoro lose those entries when running disable.

Expected behavior

  • mihoro cron enable should add or update the mihoro entry while preserving all other entries.
  • mihoro cron disable should remove only the mihoro entry, leaving other entries intact.

Suggested fix

Read the current crontab via `crontab -l`, splice in/out the mihoro line, and write back:

fn read_current_crontab() -> Result<String> {
    let output = Command::new(\"crontab\").arg(\"-l\").output()?;
    if output.status.success() {
        Ok(String::from_utf8_lossy(&output.stdout).into_owned())
    } else {
        Ok(String::new())  // no existing crontab is fine
    }
}

pub fn enable_auto_update(interval_hours: u16, prefix: &str) -> Result<()> {
    let new_entry = generate_cron_entry(interval_hours)?;
    let bin_path = mihoro_bin_path()?;
    let marker = format!(\" {} update\", bin_path);

    let existing = read_current_crontab()?;
    let mut kept: Vec<&str> = existing
        .lines()
        .filter(|l| !l.contains(&marker))
        .collect();
    let trimmed = new_entry.trim_end_matches('\n').to_string();
    kept.push(&trimmed);
    let merged = kept.join(\"\n\") + \"\n\";

    let crontab_file = crontab_path();
    fs::write(&crontab_file, merged)?;

    let status = Command::new(\"crontab\").arg(&crontab_file).status()?;
    // ...
}

Same pattern (filter out, write back) for `disable_auto_update` instead of `crontab -r`.

Severity

Data loss — silent destruction of unrelated user state, no warning, no backup. Anyone who runs `mihoro cron enable` on a server with existing cron jobs (web hosting panels, backup automation, certificate renewal, etc.) will have those jobs silently dropped, and may not notice until the missing job's effects are felt (e.g. expired certificate, missed backup).

Workaround

Until fixed, users should back up before invoking either command:

crontab -l > /tmp/cron.bak
mihoro cron enable
# manually merge /tmp/cron.bak entries back in

Or skip `mihoro cron` entirely and add the entry manually:

(crontab -l 2>/dev/null; echo '0 */12 * * * /root/.local/bin/mihoro update') | crontab -

Happy to send a PR if you'd like.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions