* fix(security): reject traversal-shaped job_id in cron output endpoint (#3661) Co-authored-by: hinotoi-agent <paperlantern.agent@gmail.com> * docs(changelog): v0.51.273 — Release IO (stage-p3b) * test(cron): guard new cron-output tests with @requires_agent_modules (#3661) The two new direct-handler tests import cron.jobs, which lives in hermes-agent and is NOT installed in CI — without the marker they error/hang in the no-agent CI shard (caught by the shard-0 timeout). Mirrors how the other 30 agent-dependent tests skip cleanly when hermes-agent modules aren't importable. --------- Co-authored-by: nesquena-hermes <[email protected]> Co-authored-by: hinotoi-agent <paperlantern.agent@gmail.com>
This commit is contained in:
3 files changed
+79
No files matched your search
@@ -3,6 +3,11 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## [v0.51.273] — 2026-06-05 — Release IO (stage-p3b — cron-output traversal guard)
|
||||
|
||||
### Fixed
|
||||
- **The cron-output endpoint rejects traversal-shaped `job_id` values** (`/api/crons/output?job_id=…`) before resolving paths or globbing markdown, matching the boundary already enforced by the cron history/detail handlers. Prevents reading markdown files outside the cron output directory. (#3661, @rodboev)
|
||||
|
||||
## [v0.51.272] — 2026-06-05 — Release IN (stage-p3a — conflict-safe self-update recovery)
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -10484,11 +10484,17 @@ def _cron_output_snippet(text: str, limit: int = 600) -> str:
|
||||
|
||||
def _handle_cron_output(handler, parsed):
|
||||
from cron.jobs import OUTPUT_DIR as CRON_OUT
|
||||
import re as _re
|
||||
|
||||
qs = parse_qs(parsed.query)
|
||||
job_id = qs.get("job_id", [""])[0]
|
||||
if not job_id:
|
||||
return j(handler, {"error": "job_id required"}, status=400)
|
||||
# Match the job_id boundary enforced by the newer cron history/detail
|
||||
# handlers. This endpoint also builds CRON_OUT / job_id before globbing
|
||||
# markdown outputs, so reject traversal-shaped IDs before path resolution.
|
||||
if not _re.fullmatch(r"[A-Za-z0-9_-][A-Za-z0-9_.-]{0,63}", job_id):
|
||||
return j(handler, {"error": "invalid job_id"}, status=400)
|
||||
# Reject malformed limit instead of letting int() raise ValueError and
|
||||
# surface as a confusing 500. Clamp to a safe range; a negative value must
|
||||
# never reach the slice below — files is sorted newest-first, so a negative
|
||||
|
||||
@@ -2,6 +2,10 @@
|
||||
Sprint 10 Tests: server.py split, cancel endpoint, cron history, tool card polish.
|
||||
"""
|
||||
import json, pathlib, urllib.error, urllib.request, urllib.parse
|
||||
from io import BytesIO
|
||||
|
||||
from tests.conftest import requires_agent_modules
|
||||
|
||||
REPO_ROOT = pathlib.Path(__file__).parent.parent.resolve()
|
||||
|
||||
from tests._pytest_port import BASE
|
||||
@@ -24,6 +28,25 @@ def post(path, body=None):
|
||||
except urllib.error.HTTPError as e:
|
||||
return json.loads(e.read()), e.code
|
||||
|
||||
|
||||
class CaptureHandler:
|
||||
headers = {}
|
||||
|
||||
def __init__(self):
|
||||
self.status = None
|
||||
self.response_headers = []
|
||||
self.wfile = BytesIO()
|
||||
|
||||
def send_response(self, status):
|
||||
self.status = status
|
||||
|
||||
def send_header(self, name, value):
|
||||
self.response_headers.append((name, value))
|
||||
|
||||
def end_headers(self):
|
||||
pass
|
||||
|
||||
|
||||
def make_session(created_list):
|
||||
d, _ = post("/api/session/new", {})
|
||||
sid = d["session"]["session_id"]
|
||||
@@ -104,6 +127,51 @@ def test_crons_output_limit_param(cleanup_test_sessions):
|
||||
# 404 or 200 with empty -- both acceptable for nonexistent job
|
||||
assert status in (200, 404)
|
||||
|
||||
|
||||
@requires_agent_modules
|
||||
def test_crons_output_rejects_traversal_job_id(monkeypatch, tmp_path):
|
||||
"""Cron output listing must not read markdown files outside OUTPUT_DIR."""
|
||||
from api.routes import _handle_cron_output
|
||||
import cron.jobs
|
||||
|
||||
output_dir = tmp_path / "cron" / "output"
|
||||
secret_dir = tmp_path / "memories"
|
||||
output_dir.mkdir(parents=True)
|
||||
secret_dir.mkdir()
|
||||
(secret_dir / "MEMORY.md").write_text("SECRET_MARKDOWN_TOKEN\n", encoding="utf-8")
|
||||
monkeypatch.setattr(cron.jobs, "OUTPUT_DIR", output_dir)
|
||||
|
||||
handler = CaptureHandler()
|
||||
parsed = urllib.parse.urlparse("/api/crons/output?job_id=../../memories&limit=5")
|
||||
_handle_cron_output(handler, parsed)
|
||||
|
||||
body = json.loads(handler.wfile.getvalue().decode("utf-8"))
|
||||
assert handler.status == 400
|
||||
assert body == {"error": "invalid job_id"}
|
||||
assert "SECRET_MARKDOWN_TOKEN" not in handler.wfile.getvalue().decode("utf-8")
|
||||
|
||||
|
||||
@requires_agent_modules
|
||||
def test_crons_output_still_returns_valid_job_outputs(monkeypatch, tmp_path):
|
||||
"""Valid job ids still list recent markdown output content."""
|
||||
from api.routes import _handle_cron_output
|
||||
import cron.jobs
|
||||
|
||||
output_dir = tmp_path / "cron" / "output"
|
||||
job_dir = output_dir / "job_123"
|
||||
job_dir.mkdir(parents=True)
|
||||
(job_dir / "run.md").write_text("# Cron Job\n\n## Response\nexpected output\n", encoding="utf-8")
|
||||
monkeypatch.setattr(cron.jobs, "OUTPUT_DIR", output_dir)
|
||||
|
||||
handler = CaptureHandler()
|
||||
parsed = urllib.parse.urlparse("/api/crons/output?job_id=job_123&limit=5")
|
||||
_handle_cron_output(handler, parsed)
|
||||
|
||||
body = json.loads(handler.wfile.getvalue().decode("utf-8"))
|
||||
assert handler.status == 200
|
||||
assert body["job_id"] == "job_123"
|
||||
assert body["outputs"] == [{"filename": "run.md", "content": "# Cron Job\n\n## Response\nexpected output\n"}]
|
||||
|
||||
def test_cron_history_button_in_panels_js(cleanup_test_sessions):
|
||||
src, _ = get_text("/static/panels.js")
|
||||
# After the main-view refactor, cron runs load inline into the detail card
|
||||
|
||||
Reference in new issue
Block a user