diff --git a/cron/jobs.py b/cron/jobs.py index e1b775bb4b505..c4d06d30e203b 100644 --- a/cron/jobs.py +++ b/cron/jobs.py @@ -370,6 +370,44 @@ def _jobs_lock(): # into output writes/deletes. _IMMUTABLE_JOB_FIELDS = frozenset({"id"}) +# Fields a caller may legitimately change via update_job. Everything else — +# including typos like ``promt`` — is rejected loudly instead of being +# silently persisted to jobs.json and reported as a successful update +# (#67625). Mirrors the create_job() parameter set plus schedule_display +# (which update_job derives itself) and the scheduler-owned lifecycle fields +# that pause_job/resume_job/trigger_job persist through update_job. Other +# runtime fields (last_run_at, last_status, run_claim, ...) are owned by +# the scheduler and never updatable. +_UPDATEABLE_JOB_FIELDS = frozenset( + { + "prompt", + "schedule", + "name", + "repeat", + "deliver", + "origin", + "skill", + "skills", + "model", + "provider", + "base_url", + "script", + "context_from", + "description", + "enabled_toolsets", + "workdir", + "no_agent", + "attach_to_session", + "schedule_display", + # lifecycle fields written by pause/resume/trigger + "enabled", + "state", + "paused_at", + "paused_reason", + "next_run_at", + } +) + def _job_output_dir(job_id: str) -> Path: """Resolve a job's output directory, rejecting any path-escape attempt. @@ -1515,6 +1553,14 @@ def update_job(job_id: str, updates: Dict[str, Any]) -> Optional[Dict[str, Any]] f"Cron job field(s) cannot be updated: {', '.join(sorted(bad_fields))}" ) + unknown_fields = (updates or {}).keys() - _UPDATEABLE_JOB_FIELDS + if unknown_fields: + raise ValueError( + "Cron job update contains unknown field(s): " + f"{', '.join(sorted(unknown_fields))}. Known updateable fields: " + f"{', '.join(sorted(_UPDATEABLE_JOB_FIELDS))}" + ) + with _jobs_lock(): jobs = load_jobs() for i, job in enumerate(jobs): diff --git a/tests/cron/test_jobs.py b/tests/cron/test_jobs.py index 6bfb7edffdf23..d06fdc5a9f57a 100644 --- a/tests/cron/test_jobs.py +++ b/tests/cron/test_jobs.py @@ -251,6 +251,32 @@ def test_update_name(self, tmp_cron_dir): fetched = get_job(job["id"]) assert fetched["name"] == "New Name" + def test_update_rejects_unknown_fields(self, tmp_cron_dir): + """#67625: a typo'd field like ``promt`` must be rejected loudly + instead of silently persisted as a successful update.""" + job = create_job(prompt="Check server status", schedule="every 1h", name="Old Name") + with pytest.raises(ValueError, match="unknown field.*promt"): + update_job(job["id"], {"promt": "some value"}) + # Nothing was persisted + fetched = get_job(job["id"]) + assert "promt" not in fetched + assert fetched["prompt"] == "Check server status" + + def test_update_accepts_all_known_fields(self, tmp_cron_dir): + """Every create_job parameter is a legal update key.""" + job = create_job(prompt="P", schedule="30m") + updates = { + "prompt": "P2", "schedule": "1h", "name": "n", "repeat": 2, + "deliver": "local", "origin": {"platform": "test"}, + "skill": "s1", "skills": ["s1", "s2"], "model": "m", "provider": "p", + "base_url": "http://x", "script": "", "context_from": None, + "enabled_toolsets": ["web"], "workdir": None, "no_agent": False, + "attach_to_session": None, "schedule_display": "1h", + } + updated = update_job(job["id"], updates) + assert updated is not None + assert updated["name"] == "n" + class TestPauseResumeJob: def test_pause_sets_state(self, tmp_cron_dir):