Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions cron/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 —
Comment thread
Enough1122 marked this conversation as resolved.
# 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.
Expand Down Expand Up @@ -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):
Expand Down
26 changes: 26 additions & 0 deletions tests/cron/test_jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Loading