mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-28 01:21:43 +00:00
fix(api-server): harden jobs API — input limits, field whitelist, startup check, tests
Five improvements to the /api/jobs endpoints: 1. Startup availability check — cron module imported once at class load, endpoints return 501 if unavailable (not 500 per-request import error) 2. Input limits — name ≤ 200 chars, prompt ≤ 5000 chars, repeat must be positive int 3. Update field whitelist — only name/schedule/prompt/deliver/skills/ repeat/enabled pass through to cron.jobs.update_job, preventing arbitrary key injection 4. Deduplicated validation — _check_job_id and _check_jobs_available helpers replace repeated boilerplate 5. 32 new tests covering all endpoints, validation, auth, and cron-unavailable cases
This commit is contained in:
Teknium
parent
57d3ac0c0b
commit
0f1c970179
2 changed files with 710 additions and 48 deletions
|
|
@ -691,23 +691,57 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
# Cron jobs API
|
||||
# ------------------------------------------------------------------
|
||||
|
||||
@staticmethod
|
||||
def _validate_job_id(job_id: str) -> Optional[str]:
|
||||
"""Return error message if job_id is invalid, else None."""
|
||||
import re as _re
|
||||
if not _re.fullmatch(r"[a-f0-9]{12}", job_id):
|
||||
return "Invalid job ID format"
|
||||
# Check cron module availability once (not per-request)
|
||||
_CRON_AVAILABLE = False
|
||||
try:
|
||||
from cron.jobs import (
|
||||
list_jobs as _cron_list,
|
||||
get_job as _cron_get,
|
||||
create_job as _cron_create,
|
||||
update_job as _cron_update,
|
||||
remove_job as _cron_remove,
|
||||
pause_job as _cron_pause,
|
||||
resume_job as _cron_resume,
|
||||
trigger_job as _cron_trigger,
|
||||
)
|
||||
_CRON_AVAILABLE = True
|
||||
except ImportError:
|
||||
pass
|
||||
|
||||
_JOB_ID_RE = __import__("re").compile(r"[a-f0-9]{12}")
|
||||
# Allowed fields for update — prevents clients injecting arbitrary keys
|
||||
_UPDATE_ALLOWED_FIELDS = {"name", "schedule", "prompt", "deliver", "skills", "skill", "repeat", "enabled"}
|
||||
_MAX_NAME_LENGTH = 200
|
||||
_MAX_PROMPT_LENGTH = 5000
|
||||
|
||||
def _check_jobs_available(self) -> Optional["web.Response"]:
|
||||
"""Return error response if cron module isn't available."""
|
||||
if not self._CRON_AVAILABLE:
|
||||
return web.json_response(
|
||||
{"error": "Cron module not available"}, status=501,
|
||||
)
|
||||
return None
|
||||
|
||||
def _check_job_id(self, request: "web.Request") -> tuple:
|
||||
"""Validate and extract job_id. Returns (job_id, error_response)."""
|
||||
job_id = request.match_info["job_id"]
|
||||
if not self._JOB_ID_RE.fullmatch(job_id):
|
||||
return job_id, web.json_response(
|
||||
{"error": "Invalid job ID format"}, status=400,
|
||||
)
|
||||
return job_id, None
|
||||
|
||||
async def _handle_list_jobs(self, request: "web.Request") -> "web.Response":
|
||||
"""GET /api/jobs — list all cron jobs."""
|
||||
auth_err = self._check_auth(request)
|
||||
if auth_err:
|
||||
return auth_err
|
||||
cron_err = self._check_jobs_available()
|
||||
if cron_err:
|
||||
return cron_err
|
||||
try:
|
||||
from cron.jobs import list_jobs
|
||||
include_disabled = request.query.get("include_disabled", "").lower() in ("true", "1")
|
||||
jobs = list_jobs(include_disabled=include_disabled)
|
||||
jobs = self._cron_list(include_disabled=include_disabled)
|
||||
return web.json_response({"jobs": jobs})
|
||||
except Exception as e:
|
||||
return web.json_response({"error": str(e)}, status=500)
|
||||
|
|
@ -717,11 +751,13 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
auth_err = self._check_auth(request)
|
||||
if auth_err:
|
||||
return auth_err
|
||||
cron_err = self._check_jobs_available()
|
||||
if cron_err:
|
||||
return cron_err
|
||||
try:
|
||||
from cron.jobs import create_job
|
||||
body = await request.json()
|
||||
name = body.get("name", "").strip()
|
||||
schedule = body.get("schedule", "").strip()
|
||||
name = (body.get("name") or "").strip()
|
||||
schedule = (body.get("schedule") or "").strip()
|
||||
prompt = body.get("prompt", "")
|
||||
deliver = body.get("deliver", "local")
|
||||
skills = body.get("skills")
|
||||
|
|
@ -729,8 +765,18 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
|
||||
if not name:
|
||||
return web.json_response({"error": "Name is required"}, status=400)
|
||||
if len(name) > self._MAX_NAME_LENGTH:
|
||||
return web.json_response(
|
||||
{"error": f"Name must be ≤ {self._MAX_NAME_LENGTH} characters"}, status=400,
|
||||
)
|
||||
if not schedule:
|
||||
return web.json_response({"error": "Schedule is required"}, status=400)
|
||||
if len(prompt) > self._MAX_PROMPT_LENGTH:
|
||||
return web.json_response(
|
||||
{"error": f"Prompt must be ≤ {self._MAX_PROMPT_LENGTH} characters"}, status=400,
|
||||
)
|
||||
if repeat is not None and (not isinstance(repeat, int) or repeat < 1):
|
||||
return web.json_response({"error": "Repeat must be a positive integer"}, status=400)
|
||||
|
||||
kwargs = {
|
||||
"prompt": prompt,
|
||||
|
|
@ -743,7 +789,7 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
if repeat is not None:
|
||||
kwargs["repeat"] = repeat
|
||||
|
||||
job = create_job(**kwargs)
|
||||
job = self._cron_create(**kwargs)
|
||||
return web.json_response({"job": job})
|
||||
except Exception as e:
|
||||
return web.json_response({"error": str(e)}, status=500)
|
||||
|
|
@ -753,13 +799,14 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
auth_err = self._check_auth(request)
|
||||
if auth_err:
|
||||
return auth_err
|
||||
job_id = request.match_info["job_id"]
|
||||
err = self._validate_job_id(job_id)
|
||||
if err:
|
||||
return web.json_response({"error": err}, status=400)
|
||||
cron_err = self._check_jobs_available()
|
||||
if cron_err:
|
||||
return cron_err
|
||||
job_id, id_err = self._check_job_id(request)
|
||||
if id_err:
|
||||
return id_err
|
||||
try:
|
||||
from cron.jobs import get_job
|
||||
job = get_job(job_id)
|
||||
job = self._cron_get(job_id)
|
||||
if not job:
|
||||
return web.json_response({"error": "Job not found"}, status=404)
|
||||
return web.json_response({"job": job})
|
||||
|
|
@ -771,14 +818,28 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
auth_err = self._check_auth(request)
|
||||
if auth_err:
|
||||
return auth_err
|
||||
job_id = request.match_info["job_id"]
|
||||
err = self._validate_job_id(job_id)
|
||||
if err:
|
||||
return web.json_response({"error": err}, status=400)
|
||||
cron_err = self._check_jobs_available()
|
||||
if cron_err:
|
||||
return cron_err
|
||||
job_id, id_err = self._check_job_id(request)
|
||||
if id_err:
|
||||
return id_err
|
||||
try:
|
||||
from cron.jobs import update_job
|
||||
body = await request.json()
|
||||
job = update_job(job_id, body)
|
||||
# Whitelist allowed fields to prevent arbitrary key injection
|
||||
sanitized = {k: v for k, v in body.items() if k in self._UPDATE_ALLOWED_FIELDS}
|
||||
if not sanitized:
|
||||
return web.json_response({"error": "No valid fields to update"}, status=400)
|
||||
# Validate lengths if present
|
||||
if "name" in sanitized and len(sanitized["name"]) > self._MAX_NAME_LENGTH:
|
||||
return web.json_response(
|
||||
{"error": f"Name must be ≤ {self._MAX_NAME_LENGTH} characters"}, status=400,
|
||||
)
|
||||
if "prompt" in sanitized and len(sanitized["prompt"]) > self._MAX_PROMPT_LENGTH:
|
||||
return web.json_response(
|
||||
{"error": f"Prompt must be ≤ {self._MAX_PROMPT_LENGTH} characters"}, status=400,
|
||||
)
|
||||
job = self._cron_update(job_id, sanitized)
|
||||
if not job:
|
||||
return web.json_response({"error": "Job not found"}, status=404)
|
||||
return web.json_response({"job": job})
|
||||
|
|
@ -790,13 +851,14 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
auth_err = self._check_auth(request)
|
||||
if auth_err:
|
||||
return auth_err
|
||||
job_id = request.match_info["job_id"]
|
||||
err = self._validate_job_id(job_id)
|
||||
if err:
|
||||
return web.json_response({"error": err}, status=400)
|
||||
cron_err = self._check_jobs_available()
|
||||
if cron_err:
|
||||
return cron_err
|
||||
job_id, id_err = self._check_job_id(request)
|
||||
if id_err:
|
||||
return id_err
|
||||
try:
|
||||
from cron.jobs import remove_job
|
||||
success = remove_job(job_id)
|
||||
success = self._cron_remove(job_id)
|
||||
if not success:
|
||||
return web.json_response({"error": "Job not found"}, status=404)
|
||||
return web.json_response({"ok": True})
|
||||
|
|
@ -808,13 +870,14 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
auth_err = self._check_auth(request)
|
||||
if auth_err:
|
||||
return auth_err
|
||||
job_id = request.match_info["job_id"]
|
||||
err = self._validate_job_id(job_id)
|
||||
if err:
|
||||
return web.json_response({"error": err}, status=400)
|
||||
cron_err = self._check_jobs_available()
|
||||
if cron_err:
|
||||
return cron_err
|
||||
job_id, id_err = self._check_job_id(request)
|
||||
if id_err:
|
||||
return id_err
|
||||
try:
|
||||
from cron.jobs import pause_job
|
||||
job = pause_job(job_id)
|
||||
job = self._cron_pause(job_id)
|
||||
if not job:
|
||||
return web.json_response({"error": "Job not found"}, status=404)
|
||||
return web.json_response({"job": job})
|
||||
|
|
@ -826,13 +889,14 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
auth_err = self._check_auth(request)
|
||||
if auth_err:
|
||||
return auth_err
|
||||
job_id = request.match_info["job_id"]
|
||||
err = self._validate_job_id(job_id)
|
||||
if err:
|
||||
return web.json_response({"error": err}, status=400)
|
||||
cron_err = self._check_jobs_available()
|
||||
if cron_err:
|
||||
return cron_err
|
||||
job_id, id_err = self._check_job_id(request)
|
||||
if id_err:
|
||||
return id_err
|
||||
try:
|
||||
from cron.jobs import resume_job
|
||||
job = resume_job(job_id)
|
||||
job = self._cron_resume(job_id)
|
||||
if not job:
|
||||
return web.json_response({"error": "Job not found"}, status=404)
|
||||
return web.json_response({"job": job})
|
||||
|
|
@ -844,13 +908,14 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
auth_err = self._check_auth(request)
|
||||
if auth_err:
|
||||
return auth_err
|
||||
job_id = request.match_info["job_id"]
|
||||
err = self._validate_job_id(job_id)
|
||||
if err:
|
||||
return web.json_response({"error": err}, status=400)
|
||||
cron_err = self._check_jobs_available()
|
||||
if cron_err:
|
||||
return cron_err
|
||||
job_id, id_err = self._check_job_id(request)
|
||||
if id_err:
|
||||
return id_err
|
||||
try:
|
||||
from cron.jobs import trigger_job
|
||||
job = trigger_job(job_id)
|
||||
job = self._cron_trigger(job_id)
|
||||
if not job:
|
||||
return web.json_response({"error": "Job not found"}, status=404)
|
||||
return web.json_response({"job": job})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue