From 1129ee4e8ab9d8dba3dcb41eb97ff386429fe4b4 Mon Sep 17 00:00:00 2001 From: ZJPex <144683084+ZJPex@users.noreply.github.com> Date: Mon, 28 Sep 2026 15:13:34 +0800 Subject: [PATCH] feat: add bounded JSON syntax acceptance criteria (#5947) * feat: add bounded JSON syntax acceptance criteria * fix(subagents): authorize bounded local JSON acceptance reads * docs: keep JSON acceptance guidance within chain limit * docs: use English for JSON acceptance comments and guidance * fix(subagents): keep unauthorized JSON probes unverified --- README.md | 20 ++ .../harness/deerflow/subagents/AGENTS.md | 7 +- .../deerflow/subagents/acceptance_checks.py | 153 ++++++++- .../packages/harness/deerflow/tools/AGENTS.md | 3 + .../tools/builtins/batch_task_tool.py | 3 + .../deerflow/tools/builtins/task_tool.py | 5 + .../test_task_tool_acceptance_checklist.py | 29 ++ backend/tests/test_batch_acceptance.py | 14 + backend/tests/test_json_acceptance.py | 314 ++++++++++++++++++ 9 files changed, 538 insertions(+), 10 deletions(-) create mode 100644 backend/tests/test_json_acceptance.py diff --git a/README.md b/README.md index c30d6a78bb..d86722634b 100644 --- a/README.md +++ b/README.md @@ -1668,6 +1668,26 @@ The lead agent can spawn sub-agents on the fly — each with its own scoped cont For file acceptance criteria, an empty regular file in the shared workspace can satisfy `file: exists` and `file_written:`, including on remote sandboxes. It fails `file: non-empty` with a deterministic empty-file result. +To request JSON syntax validation, explicitly set a `task` or `batch_task` item's +`acceptance_criteria` to `["file:../outputs/report.json json-valid"]`. Checks cover only +complete UTF-8 JSON files within the shared workspace, up to **50,000 bytes**. +Valid syntax returns `holds`; empty files, syntax errors, non-UTF-8 content, or +`NaN`/`Infinity` return `does not hold`. Oversize files, incomplete reads, out-of-scope +paths, or parser resource limits return `UNVERIFIED`. +Reads are capped at 50,001 bytes to detect oversize content. Remote results are checked +for a completion marker, the read exit code, and the probed size. Remote providers lacking +the required probe tools leave the result unverified rather than falling back to full-content +reads. Concurrent writes of the same size do not yield an atomic snapshot. +Local checks authorize sandbox access before resolving paths or probing metadata, and recheck +before reading content; revoked permissions return `UNVERIFIED` without revealing existence. +Remote probes report missing files only with a searchable ancestor and in-scope canonical path; +inaccessible directories remain `UNVERIFIED`. UTF-8 BOMs are rejected. Top-level scalars, +duplicate keys, and syntactically valid large numbers can pass, without validating schemas, +fields, or business semantics. Other file criteria are unchanged, and `.json` files are not +checked automatically. This criterion standardizes verdicts, path boundaries, and read limits +instead of requiring ad hoc bash/Python checks, and makes insufficient evidence explicit. +Execution completion status and automatic retry policy are unchanged. + Content-less sub-agent final messages report `No response generated` instead of the literal text `None`. A content-less provider-error fallback reports its structured error detail when available. An ordinary `task` also receives a defensive snapshot of the dispatching run's current uploads. This lets eligible sub-agents use `list_uploaded_files` to find earlier-turn files without returning same-turn attachments as historical. Delayed or recovered `batch_task` workers leave this tool disabled because they have no valid turn-local upload boundary. diff --git a/backend/packages/harness/deerflow/subagents/AGENTS.md b/backend/packages/harness/deerflow/subagents/AGENTS.md index eb02018473..b7e95f1b66 100644 --- a/backend/packages/harness/deerflow/subagents/AGENTS.md +++ b/backend/packages/harness/deerflow/subagents/AGENTS.md @@ -1,13 +1,12 @@ ### Subagent System (`packages/harness/deerflow/subagents/`) +**JSON**: See README. Authorize before local metadata and reads; inaccessible paths stay UNVERIFIED. + Apply each subagent's prompt overlay after assembling its full SystemMessage. Registry overrides must not mutate `BUILTIN_SUBAGENTS`. Durable batch specs store overlays as JSON and restore them before execution. -**Empty remote artifacts**: Acceptance probes treat GNU `stat -c %F` labels -`regular file` and `regular empty file` as regular files. Empty files satisfy -`exists`/`file_written` but fail `non-empty`; symlink leaves, directories, and -FIFOs remain rejected. +**Remote empty files**: GNU stat's `regular file`/`regular empty file` are regular files. Empty files pass exists/file_written, fail non-empty; reject symlinks, directories, FIFOs. **Context**: Capture after validation, before setup. Keep genuine replies, even hidden clarifications; exclude framework state and unpaired calls. Mark unserializable media as omitted. diff --git a/backend/packages/harness/deerflow/subagents/acceptance_checks.py b/backend/packages/harness/deerflow/subagents/acceptance_checks.py index b7eb4814d5..c98279a6e2 100644 --- a/backend/packages/harness/deerflow/subagents/acceptance_checks.py +++ b/backend/packages/harness/deerflow/subagents/acceptance_checks.py @@ -8,6 +8,9 @@ requirement. Leaf families: +- ``file: json-valid`` — explicit UTF-8 JSON syntax check, capped at 50,000 bytes. + Read up to the cap plus one byte and verify completeness; oversize, truncated reads + and parser resource limits remain UNVERIFIED. Reject NaN/Infinity; no schema or semantic validation. - ``file: exists`` / ``file: non-empty`` — read through ``read_current_file_content`` (the ``ReadBeforeWriteMiddleware`` precedent), **scoped to the shared thread workspace**: the path must @@ -55,6 +58,9 @@ the async caller offloads the whole check with ``asyncio.to_thread``. from __future__ import annotations +import base64 +import binascii +import json import ntpath import os import posixpath @@ -86,6 +92,7 @@ _DETAIL_MAX_CHARS = 160 _PROVIDER_ERROR_PREFIX = "Error:" _FILE_LEAF_RE = re.compile(r"^file:(?P.+?)\s+(?Pexists|non-empty)$", re.IGNORECASE) +_JSON_FILE_LEAF_RE = re.compile(r"^file:(?P.+?)\s+json-valid$", re.IGNORECASE) _FILE_WRITTEN_RE = re.compile(r"^file_written:(?P.+)$", re.IGNORECASE) _TESTS_PASSED_RE = re.compile(r"^tests_passed:(?P.+)$", re.IGNORECASE) @@ -128,7 +135,7 @@ _TEST_FAIL_SHAPE_RE = re.compile( class AcceptanceLeaf(TypedDict): criterion: str # original criterion text (bounded) - family: str # file_exists | file_non_empty | file_written | tests_passed | undecidable + family: str # file_exists | file_non_empty | file_json_valid | file_written | tests_passed | undecidable checked: bool # a deterministic check ran holds: bool # checked AND the condition holds; always False when unchecked detail: str # short evidence note (bounded) @@ -152,6 +159,9 @@ def parse_file_criterion(criterion: str) -> tuple[str, str] | None: if file_match is not None: family = "file_exists" if file_match.group("mode").lower() == "exists" else "file_non_empty" return family, file_match.group("path") + json_match = _JSON_FILE_LEAF_RE.match(criterion) + if json_match is not None: + return "file_json_valid", json_match.group("path") written_match = _FILE_WRITTEN_RE.match(criterion) if written_match is not None: return "file_written", written_match.group("path") @@ -214,6 +224,24 @@ def _resolve_scoped_path(path: str, thread_data: Mapping[str, Any] | None, *, re return None +# A failed existence test can mean EACCES. Prove a searchable ancestor and +# canonical containment before reporting absence; broken symlinks stay uncertain. +_REMOTE_ABSENCE_GUARD = ( + 'if [ ! -e "$1" ]; then ' + '[ ! -L "$1" ] || { echo NONREGULAR; exit 0; }; ' + 'd=$(/usr/bin/dirname -- "$1") || { echo UNREADABLE; exit 0; }; ' + 'while [ ! -e "$d" ]; do ' + '[ ! -L "$d" ] && [ "$d" != / ] || { echo UNREADABLE; exit 0; }; ' + 'd=${d%/*}; [ -n "$d" ] || d=/; ' + "done; " + '[ -d "$d" ] && [ -x "$d" ] || { echo UNREADABLE; exit 0; }; ' + 'r=$(/usr/bin/realpath -- "$2") || { echo UNREADABLE; exit 0; }; ' + 'p=$(/usr/bin/realpath -m -- "$1") || { echo UNREADABLE; exit 0; }; ' + 'case $p in "$r"/*) echo NOFILE ;; *) echo ESCAPED ;; esac; exit 0; ' + "fi; " +) + + #: Remote size-probe script (POSIX sh, positional params: ``$1`` path, #: ``$2`` mount root). Answers a bare byte count for a regular file, or one #: of ``NOFILE`` / ``UNREADABLE`` / ``NONREGULAR`` / ``ESCAPED``. Everything @@ -230,8 +258,7 @@ def _resolve_scoped_path(path: str, thread_data: Mapping[str, Any] | None, *, re #: lands outside the canonical root (ESCAPED). GNU stat labels zero-byte #: regular files as ``regular empty file``; both regular-file labels qualify. _SIZE_PROBE_INNER_SCRIPT = ( - '[ -e "$1" ] || { echo NOFILE; exit 0; }; ' - 't=$(/usr/bin/stat -c %F -- "$1") || { echo UNREADABLE; exit 0; }; ' + _REMOTE_ABSENCE_GUARD + 't=$(/usr/bin/stat -c %F -- "$1") || { echo UNREADABLE; exit 0; }; ' 'case "$t" in "regular file"|"regular empty file") ;; *) echo NONREGULAR; exit 0 ;; esac; ' 'r=$(/usr/bin/realpath -- "$2") || { echo UNREADABLE; exit 0; }; ' 'p=$(/usr/bin/realpath -- "$1") || { echo UNREADABLE; exit 0; }; ' @@ -320,8 +347,7 @@ def _probe_file_size(runtime: Any, resolved: str, thread_data: Mapping[str, Any] #: claim. The regular-file gate runs first, so no FIFO or device is ever #: opened. _READ_PROBE_INNER_SCRIPT = ( - '[ -e "$1" ] || { echo NOFILE; exit 0; }; ' - 't=$(/usr/bin/stat -c %F -- "$1") || { echo UNREADABLE; exit 0; }; ' + _REMOTE_ABSENCE_GUARD + 't=$(/usr/bin/stat -c %F -- "$1") || { echo UNREADABLE; exit 0; }; ' 'case "$t" in "regular file"|"regular empty file") ;; *) echo NONREGULAR; exit 0 ;; esac; ' 'r=$(/usr/bin/realpath -- "$2") || { echo UNREADABLE; exit 0; }; ' 'p=$(/usr/bin/realpath -- "$1") || { echo UNREADABLE; exit 0; }; ' @@ -379,6 +405,109 @@ def _probe_file_readable(runtime: Any, resolved: str, thread_data: Mapping[str, return None +# Encode content with the read exit code; the outer marker detects provider truncation, and file content cannot forge the exit code. +_JSON_READ_INNER_SCRIPT = ( + _REMOTE_ABSENCE_GUARD + 't=$(/usr/bin/stat -c %F -- "$1") || exit 1; ' + 'case "$t" in "regular file"|"regular empty file") ;; *) exit 1 ;; esac; ' + 'r=$(/usr/bin/realpath -- "$2") || exit 1; ' + 'p=$(/usr/bin/realpath -- "$1") || exit 1; ' + 'case $p in "$r"/*) ;; *) exit 1 ;; esac; ' + 'printf "JSON\\n"; ' + '{ /usr/bin/timeout 5 /usr/bin/head -c "$3" -- "$p"; printf "\\n%03d" "$?"; } | /usr/bin/base64 -w 0; ' + 'printf "\\nEND\\n"' +) + + +def _read_bounded_json_content(runtime: Any, resolved: str, thread_data: Mapping[str, Any] | None) -> bytes | None: + """Read up to the cap plus one byte to detect growth; return None if completeness is uncertain.""" + from deerflow.authz.sandbox_authz import authorize_sandbox_execution, safe_app_config + from deerflow.sandbox.tools import _resolve_local_read_path, ensure_sandbox_initialized, is_local_sandbox + + if not is_local_sandbox(runtime): + sandbox = ensure_sandbox_initialized(runtime) + root = "/".join(resolved.split("/")[:4]) + output = sandbox.execute_command( + f"/usr/bin/env -i /bin/sh -c {shlex.quote(_JSON_READ_INNER_SCRIPT)} json-read {shlex.quote(resolved)} {shlex.quote(root)} {_FILE_CONTENT_READ_CAP_BYTES + 1}", + env={"_DEERFLOW_SIZE_PROBE": "1"}, + timeout=10, + ) + if not isinstance(output, str): + return None + output = output.strip() + if output == "NOFILE": + raise FileNotFoundError(resolved) + if not output.startswith("JSON\n") or not output.endswith("\nEND"): + return None + encoded = output[5:-4] + if len(encoded) > 4 * ((_FILE_CONTENT_READ_CAP_BYTES + 5 + 2) // 3): + return None + try: + payload = base64.b64decode(encoded, validate=True) + except (binascii.Error, ValueError): + return None + return payload[:-4] if payload.endswith(b"\n000") else None + # Reuse live sandbox authorization; persisted local sandbox IDs cannot bypass revoked permissions. + authorize_sandbox_execution(context=getattr(runtime, "context", None) or {}, app_config=safe_app_config()) + host_path = _resolve_local_read_path(resolved, thread_data) + if host_path == resolved: + return None + flags = os.O_RDONLY | getattr(os, "O_BINARY", 0) | getattr(os, "O_NONBLOCK", 0) + with os.fdopen(os.open(host_path, flags), "rb", buffering=0) as handle: + if not stat.S_ISREG(os.fstat(handle.fileno()).st_mode): + return None + content = bytearray() + # Disable buffered read-ahead; continue short reads to EOF or cap plus one to distinguish complete from truncated content. + while len(content) <= _FILE_CONTENT_READ_CAP_BYTES: + chunk = handle.read(_FILE_CONTENT_READ_CAP_BYTES + 1 - len(content)) + if not chunk: + break + content.extend(chunk) + return bytes(content) + + +class _NonStandardJSONConstant(ValueError): + """Distinguish nonstandard constants from parser resource limits.""" + + +def _reject_json_constant(value: str) -> None: + raise _NonStandardJSONConstant(value) + + +def _check_json_file(base: AcceptanceLeaf, runtime: Any, resolved: str, thread_data: Mapping[str, Any] | None, probed_size: int) -> AcceptanceLeaf: + """Check JSON syntax only for complete UTF-8 documents within the read cap.""" + from deerflow.sandbox.exceptions import SandboxError, SandboxFileNotFoundError + + if probed_size > _FILE_CONTENT_READ_CAP_BYTES: + base["detail"] = f"JSON file exceeds {_FILE_CONTENT_READ_CAP_BYTES}-byte read cap; content not read" + return base + try: + content = _read_bounded_json_content(runtime, resolved, thread_data) + except (FileNotFoundError, SandboxFileNotFoundError): + base["checked"] = True + base["detail"] = "file does not exist" + return base + except (OSError, SandboxError): + base["detail"] = "bounded JSON read failed" + return base + if content is None or len(content) > _FILE_CONTENT_READ_CAP_BYTES or len(content) != probed_size: + base["detail"] = "complete JSON content unavailable within the read cap" + return base + try: + # Validate numeric syntax without converting large integers or overflowing floating-point values. + json.loads(content.decode("utf-8"), parse_constant=_reject_json_constant, parse_int=str, parse_float=str) + except (UnicodeDecodeError, json.JSONDecodeError, _NonStandardJSONConstant): + base["checked"] = True + base["detail"] = "invalid UTF-8 JSON syntax" + return base + except (RecursionError, ValueError, MemoryError): + base["detail"] = "JSON parser resource limit; syntax unverified" + return base + base["checked"] = True + base["holds"] = True + base["detail"] = f"valid JSON syntax, {len(content)} bytes; no schema validation" + return base + + def _check_file_leaf( family: str, path: str, @@ -396,11 +525,21 @@ def _check_file_leaf( from deerflow.sandbox.exceptions import SandboxError, SandboxFileNotFoundError from deerflow.sandbox.tools import is_local_sandbox + base: AcceptanceLeaf = {"criterion": "", "family": family, "checked": False, "holds": False, "detail": ""} + if family == "file_json_valid" and is_local_sandbox(runtime): + from deerflow.authz.sandbox_authz import authorize_sandbox_execution, safe_app_config + + # Authorize before canonicalization or metadata probes can disclose file existence. + try: + authorize_sandbox_execution(context=getattr(runtime, "context", None) or {}, app_config=safe_app_config()) + except SandboxError: + base["detail"] = "sandbox access denied; JSON file unverified" + return base + # Symlink escapes are a local-sandbox concern (host-visible links); remote # providers resolve paths inside the sandbox where the parent cannot # canonicalize, so the check stays lexical there. resolved = _resolve_scoped_path(criterion_path, thread_data, resolve_symlinks=is_local_sandbox(runtime)) - base: AcceptanceLeaf = {"criterion": "", "family": family, "checked": False, "holds": False, "detail": ""} if resolved is None: base["detail"] = "path is outside the shared thread workspace" if thread_data else "shared thread workspace unavailable" return base @@ -417,6 +556,8 @@ def _check_file_leaf( # worker — degrade to UNVERIFIED instead. base["detail"] = "file size could not be established by a bounded probe; content not read" return base + if family == "file_json_valid": + return _check_json_file(base, runtime, resolved, thread_data, probed_size) if probed_size > _FILE_CONTENT_READ_CAP_BYTES: # Large deliverable: the size probe proved the file exists, is # regular, and is non-empty (size > cap > 0) — answering the diff --git a/backend/packages/harness/deerflow/tools/AGENTS.md b/backend/packages/harness/deerflow/tools/AGENTS.md index a106683ec7..e011110999 100644 --- a/backend/packages/harness/deerflow/tools/AGENTS.md +++ b/backend/packages/harness/deerflow/tools/AGENTS.md @@ -1,5 +1,8 @@ ### Tool System (`packages/harness/deerflow/tools/`) +`task` and `batch_task` opt into JSON checks with `file: json-valid`. +See [subagents/AGENTS.md](../subagents/AGENTS.md) for read limits and UNVERIFIED semantics. + `list_uploaded_files` 的续页契约见 [FILE_UPLOAD.md](../../../../docs/FILE_UPLOAD.md)。 游标绑定可信用户/线程、规范化过滤条件、本轮上传排除集合和目录元数据；身份与目录 始终由 runtime 解析，游标只负责一致性校验。先过滤，再按修改时间降序和原始文件名 diff --git a/backend/packages/harness/deerflow/tools/builtins/batch_task_tool.py b/backend/packages/harness/deerflow/tools/builtins/batch_task_tool.py index 33ff27e699..ebb0444e12 100644 --- a/backend/packages/harness/deerflow/tools/builtins/batch_task_tool.py +++ b/backend/packages/harness/deerflow/tools/builtins/batch_task_tool.py @@ -171,6 +171,9 @@ async def batch_task( separate acceptance verdict; ``succeeded`` only means execution completed. Retain useful results, repair unmet conditions, and verify consequential unknowns or preserve uncertainty. Acceptance never triggers automatic retries. + JSON deliverables can opt into ``file: json-valid``: validate complete UTF-8 + JSON syntax up to 50,000 bytes, rejecting NaN/Infinity. Oversize files or incomplete + reads return UNVERIFIED. No schema or business validation is performed. Args: title: Short batch name shown to the user. diff --git a/backend/packages/harness/deerflow/tools/builtins/task_tool.py b/backend/packages/harness/deerflow/tools/builtins/task_tool.py index a61ddc3c65..08243aeae1 100644 --- a/backend/packages/harness/deerflow/tools/builtins/task_tool.py +++ b/backend/packages/harness/deerflow/tools/builtins/task_tool.py @@ -738,6 +738,10 @@ async def task_tool( every criterion that cannot be checked deterministically is marked UNVERIFIED — never silently passed. A `holds` leaf is execution evidence, not a guarantee that the deliverable is correct. + - JSON deliverables can opt into `file: json-valid`: validate complete UTF-8 JSON + syntax up to 50,000 bytes, rejecting NaN/Infinity. Oversize files, incomplete reads, + or parser resource limits return UNVERIFIED. No schema or business validation; + .json files are not checked automatically. - `completed` means execution ended, not task acceptance. Read each criterion and retain useful work. For `does not hold`, inspect the reason and repair or recheck only the unmet condition, reusing unaffected outputs. @@ -766,6 +770,7 @@ async def task_tool( back marked UNVERIFIED. Example for a report-writing delegation: ["file:../outputs/report.md non-empty"]. Omit for open-ended exploration where no crisp acceptance condition exists. + JSON example: ["file:../outputs/report.json json-valid"]. description: Optional short (3-5 word) description of the task for logging/display. context_mode: Defaults to isolated (only the delegated prompt). Choose snapshot when relevant requirements or failed approaches are spread diff --git a/backend/tests/blocking_io/test_task_tool_acceptance_checklist.py b/backend/tests/blocking_io/test_task_tool_acceptance_checklist.py index 61ffa33cba..4b413556ea 100644 --- a/backend/tests/blocking_io/test_task_tool_acceptance_checklist.py +++ b/backend/tests/blocking_io/test_task_tool_acceptance_checklist.py @@ -157,6 +157,35 @@ async def test_acceptance_checklist_file_leaf_is_offloaded(monkeypatch, tmp_path assert verdict["leaves"][0]["holds"] is True +@pytest.mark.parametrize("permitted", [True, False]) +@pytest.mark.parametrize("exists", [True, False]) +async def test_json_acceptance_check_is_offloaded(monkeypatch, tmp_path, permitted, exists): + """Ordinary delegation authorizes JSON reads in a worker thread and reports an unverified verdict on denial.""" + from deerflow.authz.rbac import RbacAuthorizationProvider + from deerflow.config.authorization_config import AuthorizationConfig + + runtime = _runtime(tmp_path) + provider = RbacAuthorizationProvider(roles={"user": {"sandbox": {"allow": "*" if permitted else []}}}) + config = SimpleNamespace(authorization=AuthorizationConfig(enabled=True)) + monkeypatch.setattr("deerflow.authz.sandbox_authz.resolve_authorization_provider", lambda config: provider) + monkeypatch.setattr("deerflow.config.get_app_config", lambda: config) + runtime.context.update(user_id="user-1", user_role="user") + if exists: + (tmp_path / "user-data" / "outputs" / "report.json").write_bytes(b'{"ok": true}') + _patch_task_tool_boundary(monkeypatch, tmp_path) + command = await task_tool_module.task_tool.coroutine( + runtime=runtime, + prompt="p", + subagent_type="general-purpose", + tool_call_id="tc-json-check", + acceptance_criteria=["file:../outputs/report.json json-valid"], + ) + verdict = command.update["messages"][0].additional_kwargs["subagent_acceptance_verdict"] + assert verdict["all_hold"] is (permitted and exists) + assert verdict["leaves"][0]["family"] == "file_json_valid" + assert verdict["leaves"][0]["checked"] is permitted + + async def test_blocking_probe_reader_actually_trips_the_gate(monkeypatch, tmp_path): """Meta-check: the same read on the event loop must raise BlockingError, so the anchor above cannot go vacuously green. (The size probe is diff --git a/backend/tests/test_batch_acceptance.py b/backend/tests/test_batch_acceptance.py index c01e1f9502..117195128d 100644 --- a/backend/tests/test_batch_acceptance.py +++ b/backend/tests/test_batch_acceptance.py @@ -157,6 +157,20 @@ async def test_completed_batch_records_mixed_checks_and_exports_after_reopen(env assert output.read_text() == "Actual report" +@pytest.mark.asyncio +async def test_json_criterion_survives_batch_execution_and_storage(env): + """Batches share file admission and acceptance checks, storing execution status separately from syntax verdicts.""" + output = env.paths.sandbox_outputs_dir("thread-1", user_id="user-1") / "report.json" + output.write_bytes(b'{"incomplete":') + batch = await _submit(env, ["file:../outputs/report.json json-valid"]) + await _execute(env) + item = (await env.repo.list_items(batch["id"], user_id="user-1"))[0] + assert item["status"] == "succeeded" + leaf = item["acceptance_verdict"]["leaves"][0] + assert (leaf["family"], leaf["checked"], leaf["holds"]) == ("file_json_valid", True, False) + assert item["attempt"] == 1 + + @pytest.mark.asyncio async def test_no_criteria_skips_checker_and_preserves_success(env, monkeypatch): checker = AsyncMock(side_effect=AssertionError("no checklist")) diff --git a/backend/tests/test_json_acceptance.py b/backend/tests/test_json_acceptance.py new file mode 100644 index 0000000000..9d2c3a6ab1 --- /dev/null +++ b/backend/tests/test_json_acceptance.py @@ -0,0 +1,314 @@ +"""Explicit JSON file criteria through the public acceptance entry point.""" + +import base64 +import os +import shlex +import subprocess +import sys +from types import SimpleNamespace +from unittest.mock import Mock + +import pytest + +from deerflow.subagents.acceptance_checks import check_acceptance_criteria, parse_file_criterion + + +@pytest.mark.parametrize("sandbox_id", ["local", "local:thread-1"]) +@pytest.mark.parametrize("content", [b"{}", b"{invalid", None]) +def test_local_json_rechecks_revoked_sandbox_grant(tmp_path, monkeypatch, sandbox_id, content): + """Retained sandbox IDs do not retain grants; revocation prevents file reads and syntax verdicts.""" + from deerflow.authz.rbac import RbacAuthorizationProvider + from deerflow.config.authorization_config import AuthorizationConfig + + config = SimpleNamespace(authorization=AuthorizationConfig(enabled=True)) + provider = RbacAuthorizationProvider(roles={"user": {"sandbox": {"allow": "*"}}}) + monkeypatch.setattr("deerflow.config.get_app_config", lambda: config) + monkeypatch.setattr("deerflow.authz.sandbox_authz.resolve_authorization_provider", lambda config: provider) + runtime = SimpleNamespace( + state={"sandbox": {"sandbox_id": sandbox_id}}, + context={"thread_id": "thread-1", "user_id": "user-1", "user_role": "user"}, + ) + if content is not None: + (tmp_path / "report.json").write_bytes(content) + criterion = "file:report.json json-valid" + + def check(): + return check_acceptance_criteria([criterion], runtime=runtime, thread_data={"workspace_path": str(tmp_path)}) + + assert check()["leaves"][0]["checked"] is True + provider = RbacAuthorizationProvider(roles={"user": {"sandbox": {"allow": []}}}) + open_file = Mock(wraps=os.open) + monkeypatch.setattr(os, "open", open_file) + verdict = check() + open_file.assert_not_called() + assert (verdict["leaves"][0]["checked"], verdict["leaves"][0]["holds"]) == (False, False) + assert verdict["unchecked"] == [criterion] + assert runtime.state["sandbox"]["sandbox_id"] == sandbox_id + + provider = RbacAuthorizationProvider(roles={"user": {"sandbox": {"allow": "*"}}}) + restored = check()["leaves"][0] + assert (restored["checked"], restored["holds"]) == (True, content == b"{}") + if content is None: + open_file.assert_not_called() + else: + open_file.assert_called_once() + + +def test_json_file_is_checked_explicitly(tmp_path): + workspace = tmp_path / "workspace" + workspace.mkdir() + (workspace / "report.json").write_bytes(b'{"items": [1, true, null]}') + runtime = SimpleNamespace(state={"sandbox": {"sandbox_id": "local"}}) + criterion = "file:report.json json-valid" + verdict = check_acceptance_criteria([criterion], runtime=runtime, thread_data={"workspace_path": str(workspace)}) + + assert parse_file_criterion(criterion) == ("file_json_valid", "report.json") + assert verdict["leaves"][0]["checked"] is True + assert verdict["leaves"][0]["holds"] is True + assert verdict["all_hold"] is True + + +@pytest.mark.parametrize("content", [b"NaN", b"Infinity", b"-Infinity", b'{"value": NaN}']) +def test_nonstandard_constants_do_not_hold(tmp_path, content): + (tmp_path / "report.json").write_bytes(content) + verdict = check_acceptance_criteria( + ["file:report.json json-valid"], + runtime=SimpleNamespace(state={"sandbox": {"sandbox_id": "local"}}), + thread_data={"workspace_path": str(tmp_path)}, + ) + assert (verdict["leaves"][0]["checked"], verdict["leaves"][0]["holds"]) == (True, False) + + +@pytest.fixture +def remote_sandbox(tmp_path, monkeypatch): + if sys.platform != "linux": + pytest.skip("Requires Linux sandbox tools") + + class ShellSandbox: + transform = staticmethod(lambda output: output) + before_read = staticmethod(lambda: None) + + def execute_command(self, command, **kwargs): + assert kwargs["env"] + args = shlex.split(command) + if "json-read" in args: + self.before_read() + prefix = "/mnt/user-data/workspace" + args = [str(tmp_path) + arg[len(prefix) :] if arg == prefix or arg.startswith(prefix + "/") else arg for arg in args] + output = subprocess.run(args, capture_output=True, text=True, check=True, timeout=15).stdout + return self.transform(output) + + def read_file(self, *args, **kwargs): + pytest.fail("JSON acceptance must not fall back to unbounded full-content reads") + + sandbox = ShellSandbox() + monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime=None: sandbox) + return sandbox + + +def test_remote_complete_json_is_checked(tmp_path, remote_sandbox): + (tmp_path / "report.json").write_bytes(b'{"ok": true}') + verdict = check_acceptance_criteria(["file:report.json json-valid"], runtime=SimpleNamespace(state=None), thread_data={"workspace_path": str(tmp_path)}) + assert verdict["all_hold"] is True + + +@pytest.fixture(params=["local", "remote"]) +def check_file(request, tmp_path): + remote = request.param == "remote" + if remote: + request.getfixturevalue("remote_sandbox") + runtime = SimpleNamespace(state=None if remote else {"sandbox": {"sandbox_id": "local"}}) + + def check(content, *, criterion="file:report.json json-valid", **kwargs): + if content is not None: + (tmp_path / "report.json").write_bytes(content) + return check_acceptance_criteria([criterion], runtime=runtime, thread_data={"workspace_path": str(tmp_path)}, **kwargs) + + return check + + +@pytest.mark.parametrize("content", [b"{}", b"[]", b"null", b"true", b"42", b'"text"', b'{"x":1,"x":2}', b"1e400", b"9" * 5000, ' {"城市":"深圳"}\r\n'.encode()]) +def test_valid_json_syntax(check_file, content): + assert check_file(content)["all_hold"] is True + + +@pytest.mark.parametrize("content", [b"", b" \r\n", b"{", b'{"a":}', b"[1,]", b"{} {}", b"//comment\n{}", b"\xff", b"\xef\xbb\xbf{}", b"NaN", b"Infinity", b"-Infinity", b'"a\x00b"']) +def test_invalid_complete_json_does_not_hold(check_file, content): + verdict = check_file(content) + assert (verdict["leaves"][0]["checked"], verdict["leaves"][0]["holds"]) == (True, False) + assert verdict["unchecked"] == [] + + +@pytest.mark.parametrize(("size", "checked", "holds"), [(49_999, True, True), (50_000, True, True), (50_001, False, False)]) +def test_byte_limit(check_file, size, checked, holds): + verdict = check_file(b'"' + b"x" * (size - 2) + b'"') + assert (verdict["leaves"][0]["checked"], verdict["leaves"][0]["holds"]) == (checked, holds) + + +def test_multibyte_content_counts_bytes(check_file): + verdict = check_file(('"' + "中" * 16_667 + '"').encode()) + assert verdict["leaves"][0]["checked"] is False + + +def test_missing_file_is_negative(check_file): + verdict = check_file(None) + assert (verdict["leaves"][0]["checked"], verdict["leaves"][0]["holds"]) == (True, False) + + +def test_parser_depth_limit_is_uncertain(check_file): + # CPython 3.12's C parser limit differs from the boundary set by sys.setrecursionlimit. + verdict = check_file(b"[" * 20_000 + b"0" + b"]" * 20_000) + assert verdict["leaves"][0]["checked"] is False + assert "resource limit" in verdict["leaves"][0]["detail"] + + +@pytest.mark.parametrize("size", [None, 1, 3]) +def test_unknown_size_or_incomplete_read_is_uncertain(check_file, size): + verdict = check_file(b"{}", size_prober=lambda *args: size) + assert verdict["leaves"][0]["checked"] is False + + +def test_growth_past_limit_is_uncertain(check_file): + verdict = check_file(b"{}" + b" " * 60_000, size_prober=lambda *args: 2) + assert verdict["leaves"][0]["checked"] is False + + +@pytest.mark.parametrize("path", ["../outside.json", "/mnt/user-data/uploads/report.json", "/mnt/user-data/workspace/../../other/report.json"]) +def test_out_of_scope_is_uncertain(check_file, path): + verdict = check_file(b"{}", criterion=f"file:{path} json-valid") + assert verdict["leaves"][0]["checked"] is False + assert "outside" in verdict["leaves"][0]["detail"] + + +def test_missing_thread_context_is_uncertain(): + verdict = check_acceptance_criteria(["file:report.json json-valid"]) + assert verdict["leaves"][0]["checked"] is False + + +def test_directory_is_uncertain(check_file, tmp_path): + (tmp_path / "report.json").mkdir() + assert check_file(None)["leaves"][0]["checked"] is False + + +@pytest.mark.skipif(os.name == "nt", reason="Requires POSIX symlinks") +def test_symlink_escape_is_uncertain(check_file, tmp_path): + outside = tmp_path.parent / "outside.json" + outside.write_bytes(b"{}") + (tmp_path / "report.json").symlink_to(outside) + assert check_file(None)["leaves"][0]["checked"] is False + + +@pytest.mark.skipif(os.name == "nt", reason="Requires POSIX FIFOs") +def test_fifo_is_uncertain_without_opening(check_file, tmp_path): + os.mkfifo(tmp_path / "report.json") + assert check_file(None)["leaves"][0]["checked"] is False + + +def test_remote_incomplete_transport_is_uncertain(tmp_path, remote_sandbox): + (tmp_path / "report.json").write_bytes(b"{}") + remote_sandbox.transform = lambda text: text[:-4] if text.startswith("JSON\n") else text + verdict = check_acceptance_criteria(["file:report.json json-valid"], runtime=SimpleNamespace(state=None), thread_data={"workspace_path": str(tmp_path)}) + assert verdict["leaves"][0]["checked"] is False + + +@pytest.mark.parametrize("output", ["Error: permission denied", "JSON\n!invalid!\nEND\n", "JSON\ne30=\nEND\n", "JSON\n" + base64.b64encode(b"{}\n001").decode() + "\nEND\n"]) +def test_remote_failed_read_is_not_invalid_json(tmp_path, remote_sandbox, output): + (tmp_path / "report.json").write_bytes(b"{}") + remote_sandbox.transform = lambda text: output if text.startswith("JSON\n") else text + verdict = check_acceptance_criteria(["file:report.json json-valid"], runtime=SimpleNamespace(state=None), thread_data={"workspace_path": str(tmp_path)}) + assert verdict["leaves"][0]["checked"] is False + + +def test_local_permission_failure_is_uncertain(tmp_path, monkeypatch): + (tmp_path / "report.json").write_bytes(b"{}") + + def denied(*args, **kwargs): + raise PermissionError("denied") + + monkeypatch.setattr(os, "open", denied) + verdict = check_acceptance_criteria(["file:report.json json-valid"], runtime=SimpleNamespace(state={"sandbox": {"sandbox_id": "local"}}), thread_data={"workspace_path": str(tmp_path)}) + assert verdict["leaves"][0]["checked"] is False + + +@pytest.mark.parametrize("criterion", ["file:report.json exists", "file:report.json non-empty", "file_written:report.json"]) +def test_json_extension_does_not_change_old_rules(check_file, criterion): + verdict = check_file(b"not JSON", criterion=criterion, content_reader=lambda *args: "not JSON") + assert verdict["all_hold"] is True + + +@pytest.mark.parametrize("criterion", ["file:report.json JSON-VALID", "FILE:report.json jſon-valid", "file:/mnt/user-data/workspace/report.json json-valid"]) +def test_normalized_spellings_use_same_json_family(check_file, criterion): + verdict = check_file(b"{}", criterion=criterion) + assert verdict["all_hold"] is True + assert verdict["leaves"][0]["family"] == "file_json_valid" + + +def test_remote_oversize_does_not_read_content(tmp_path, remote_sandbox): + (tmp_path / "report.json").write_bytes(b" " * 50_001) + remote_sandbox.before_read = lambda: pytest.fail("Oversize files must not have their content read") + verdict = check_acceptance_criteria(["file:report.json json-valid"], runtime=SimpleNamespace(state=None), thread_data={"workspace_path": str(tmp_path)}) + assert verdict["leaves"][0]["checked"] is False + + +def test_remote_file_grows_after_size_probe(tmp_path, remote_sandbox): + path = tmp_path / "report.json" + path.write_bytes(b"{}") + remote_sandbox.before_read = lambda: path.write_bytes(b"{}" + b" " * 100_000) + verdict = check_acceptance_criteria(["file:report.json json-valid"], runtime=SimpleNamespace(state=None), thread_data={"workspace_path": str(tmp_path)}) + assert verdict["leaves"][0]["checked"] is False + + +@pytest.mark.skipif(os.name == "nt" or os.geteuid() == 0, reason="Requires non-root POSIX permissions") +def test_remote_permission_denied_is_uncertain(tmp_path, remote_sandbox): + path = tmp_path / "report.json" + path.write_bytes(b"{}") + path.chmod(0) + try: + verdict = check_acceptance_criteria(["file:report.json json-valid"], runtime=SimpleNamespace(state=None), thread_data={"workspace_path": str(tmp_path)}) + assert verdict["leaves"][0]["checked"] is False + finally: + path.chmod(0o600) + + +@pytest.mark.skipif(os.name == "nt" or os.geteuid() == 0, reason="Requires non-root POSIX permissions") +@pytest.mark.parametrize(("before_read", "exists"), [(False, True), (False, False), (True, True)]) +def test_remote_inaccessible_parent_is_uncertain(tmp_path, remote_sandbox, before_read, exists): + parent = tmp_path / "locked" + nested = parent / "nested" + nested.mkdir(parents=True) + if exists: + (nested / "report.json").write_bytes(b"{}") + if before_read: + remote_sandbox.before_read = lambda: parent.chmod(0) + else: + parent.chmod(0) + try: + verdict = check_acceptance_criteria( + ["file:locked/nested/report.json json-valid"], + runtime=SimpleNamespace(state=None), + thread_data={"workspace_path": str(tmp_path)}, + ) + assert (verdict["leaves"][0]["checked"], verdict["leaves"][0]["holds"]) == (False, False) + finally: + parent.chmod(0o700) + + +@pytest.mark.parametrize("path", ["missing/report.json", "missing/nested/report.json"]) +def test_remote_missing_parent_is_negative(tmp_path, remote_sandbox, path): + verdict = check_acceptance_criteria([f"file:{path} json-valid"], runtime=SimpleNamespace(state=None), thread_data={"workspace_path": str(tmp_path)}) + assert (verdict["leaves"][0]["checked"], verdict["leaves"][0]["holds"]) == (True, False) + + +@pytest.mark.parametrize("target", ["missing", "outside"]) +def test_remote_missing_file_through_symlink_is_uncertain(tmp_path, remote_sandbox, target): + destination = tmp_path / target if target == "missing" else tmp_path.parent + (tmp_path / "link").symlink_to(destination, target_is_directory=True) + verdict = check_acceptance_criteria(["file:link/report.json json-valid"], runtime=SimpleNamespace(state=None), thread_data={"workspace_path": str(tmp_path)}) + assert verdict["leaves"][0]["checked"] is False + + +def test_remote_special_filename_is_literal(tmp_path, remote_sandbox): + filename = "report '; echo bad; #.json" + (tmp_path / filename).write_bytes(b"{}") + verdict = check_acceptance_criteria([f"file:{filename} json-valid"], runtime=SimpleNamespace(state=None), thread_data={"workspace_path": str(tmp_path)}) + assert verdict["all_hold"] is True