Files
autocad-mcp-pro/backends/quarantine.py
T
Umut CANandClaude Opus 5 ca1c79fd01
Release / test (full, windows-latest, 3.11) (push) Canceled after 0s
Release / test (pdf, ubuntu-latest, 3.11) (push) Canceled after 0s
Release / test (pdf, ubuntu-latest, 3.12) (push) Canceled after 0s
Release / build (push) Canceled after 0s
Release / publish-pypi (push) Canceled after 0s
Release / github-release (push) Canceled after 0s
fix: seven defects an adversarial audit found, and the gates that missed them
Every one of these was reported ok/clean to the caller while being wrong.

Silently wrong numbers
- selection_filter(min_area) read `entity.dxf.get("area", 0.0)` for SOLID,
  TRACE and HATCH, none of which carry that attribute, so every hatch
  measured 0.0. A 1600 mm^2 hatch was excluded by min_area=1.0 while
  `filtered_by` asserted the filter had run, and COM (which reads ActiveX
  .Area) returned a different selection for the same call. It now answers
  from the measurement `entity_measure` uses, so the two cannot disagree.
- Every headless dimension rendered through ezdxf's bare `Standard` style:
  1.0 mm text with a comma decimal marker, against ISO 3098's 2.5 mm minimum
  and ISO 129's point -- while the document header itself said $DIMTXT 2.5.
  `ezdxf.new(setup=True)` is not the fix; measured, it renders a 100 mm
  dimension as "10000". The attributes are now stored explicitly.
- Saving writes `dimrnd = 0.0` onto the Standard style, and ezdxf applies
  `xround(value, dimrnd)` whenever the attribute is present at all --
  `xround(33.333, 0.0)` is 33. So on any reopened file, including one this
  server saved, every dimension was rounded to a whole unit before DIMDEC
  formatted it: a 12.75 mm feature dimensioned 13. AutoCAD means DIMRND 0 as
  *no* rounding; the sentinel is now discarded at every load site, and a real
  0.5 mm rounding is left alone.
- The DIM* header fold gave out-of-range values teeth they did not have when
  they died in the header: dim_decimals=-1 returned ok and then killed the
  next dimension with `ValueError: Format specifier missing precision` from
  inside ezdxf, and 40 printed forty digits of float noise onto the sheet.
  Ranges are now AutoCAD's own (DIMDEC 0-8, DIMZIN 0-15, lengths positive).

Security
- security.py recorded the policy as "sanitize_command and sanitize_lisp each
  have exactly ONE caller ... the policy is scoped to the CHANNEL". The
  premise was false. `_ensure_linetype_loaded` and `ComBackend.linetype_load`
  build `_-LINETYPE _LOAD {name} {file}` and hand it to SendCommand, which
  treats a newline and ';' as segment separators -- so a `linetype=` argument
  carrying either injected further AutoCAD commands, and the 36-verb denylist
  never saw that channel. Guarded with the DXF symbol-name rule (the
  injection characters fall out of it rather than being enumerated) plus
  validate_path for the file argument, which was also the one path-taking
  tool reaching the filesystem unchecked. The policy comment is corrected.

Gates that could not see
- drawing_critique returned zero issues on a quarantined document: all seven
  checks wrapped their backend call in a bare `except Exception`. CLAUDE.md
  rule 7 makes that call the gate an agent is told to trust. Only the
  quarantine now escapes; an ordinary broken check still degrades.
  `_check_dimension_legibility` also read `_doc` directly from the event loop
  thread, missing the gate and performing exactly the unsynchronised access
  the quarantine exists to prevent. `construction_clear` answered
  "deleted: 0" -- there was nothing to clear -- for the same reason.
- The iso128 focus checked layer lineweights only, so a sheet lettered at
  1.0 mm critiqued clean. It now reads the effective dimension height and
  separator, header-then-style, in the order the renderer resolves.
- check_doc_numbers.py derived five values it never checked, and those were
  the ones that had drifted. The collected-test count (README said 1274, the
  tree had 1369) and the A/B pass-rate and Fixed columns are now gated.
- benchmarks/README.md's v1.4.0 table was impossible on its face -- 21
  passing plus 5 fixed is 26, not 24 -- and contradicted the report it
  linked, because `ab-v1.4.0-vs-v1.5.0.json` was regenerated from a tree that
  already carried the dimension fixes and so never measured the v1.5.0 tag.
  Replaced with a real run: ab-v1.4.0-vs-v1.5.1.json, 21/26 -> 26/26.

Release gate
- release.yml ran one ubuntu/3.11 lane, so a Windows-only defect could reach
  PyPI with no Windows job having executed. That is not hypothetical: v1.5.0
  shipped `except pywintypes.com_error` raising NameError wherever pywin32 is
  absent. It now runs the same three lanes as ci.yml, Windows included.

1368 passed, 1 xfailed. ruff clean. Tool inventory and all 16 gated document
numbers match the tree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-06 23:07:50 +03:00

209 lines
8.4 KiB
Python

"""Document quarantine — the vocabulary for a call that was abandoned mid-write.
``EzdxfBackend._async`` cannot cancel an ``asyncio.to_thread`` worker. When a
call overruns ``EZDXF_CALL_TIMEOUT`` the handler swaps in a fresh
``asyncio.Lock`` so later callers do not deadlock behind the orphaned one — and
that swap is precisely what removes the mutual exclusion the lock existed for.
The runaway thread keeps a live reference to the one shared ``ezdxf.Drawing``,
which is explicitly not thread-safe, through the closure it was handed.
Measured, with a runaway ``_sync`` that really appends LINEs to ``_doc``:
``entity_create_circle`` after the abandonment returned handle ``A1``;
``drawing_info`` reported 116 entities for a modelspace observed at 302 and
climbing; ``drawing_save`` returned ``{"ok": True}`` and wrote a valid DXF of
15200 of the 60000 entities the runaway would go on to write — a considered
document of a state that never existed. Only the timed-out caller was warned.
So the abandonment quarantines the DOCUMENT, not the backend. The document stays
untrusted until it is REPLACED by an object the abandoned thread has no
reference to (``drawing_open`` / ``drawing_new`` / ``drawing_close``), or until
the abandoned call is seen to finish cleanly — see ``AbandonedCall``.
This module deliberately has no ezdxf import: ``server.py`` needs the exception
type to classify a refusal on a box where a backend's dependencies are missing.
"""
from __future__ import annotations
import time
from typing import Any
__all__ = [
"AbandonedCall",
"DocumentQuarantineError",
"QuarantineRecord",
"quarantine_refusal",
"real_call_name",
]
#: Longest ``repr`` kept for a return value nobody received. A late
#: ``entity_list`` can return thousands of entities; the diagnostic is that the
#: value was lost, not what was in it.
_MAX_RESULT_REPR = 200
def real_call_name(func: Any) -> str:
"""The backend method ``func`` belongs to, not the closure's own name.
Every real backend method hands ``_async`` a closure literally named
``_sync`` — measured: ``__name__='_sync'``,
``__qualname__='EzdxfBackend.drawing_save.<locals>._sync'`` — so
``__name__`` made every production timeout read ``ezdxf call '_sync' timed
out``. The qualname carries the name the user typed and was being discarded.
"""
qualname = getattr(func, "__qualname__", "") or ""
parts = qualname.split(".")
if "<locals>" in parts:
index = parts.index("<locals>")
if index:
return parts[index - 1]
return getattr(func, "__name__", None) or "call"
class AbandonedCall:
"""Observation post for a worker thread ``_async`` can no longer await.
The obvious hook does not work, and this was checked before being relied on:
``asyncio.timeout_at`` **cancels** the Future ``_async`` awaits, so by the
time the abandon branch runs that Future is already done and
``add_done_callback`` fires immediately with ``cancelled()`` True. It reports
the *await* ending, never the thread — which ``asyncio.to_thread`` cannot
cancel. The only place the thread's outcome is observable is inside the
thread, so ``_async`` wraps every timed call in one of these.
Deliberately no ``threading.Event``: this is allocated on every ``_async``
call and an Event carries a Condition and a Lock. ``finished`` is a plain
bool written once by the worker and read from the event loop; a bool store is
atomic under the GIL and nothing here ever waits on it.
"""
__slots__ = ("func", "finished", "result", "error")
def __init__(self, func):
self.func = func
self.finished = False
self.result: Any = None
self.error: BaseException | None = None
def __call__(self, *args, **kwargs):
try:
self.result = self.func(*args, **kwargs)
return self.result
except BaseException as exc:
self.error = exc
raise
finally:
self.finished = True
class QuarantineRecord:
"""One abandoned call, and what became of it.
Holds the identity of the ``Drawing`` that was live at abandon time as well
as its path: rebinding ``self._doc`` isolates a ``_sync`` that captured
doc/msp once at entry (the shape of nearly every closure in the backend), but
one that re-reads ``self._msp()`` inside its own loop would follow the rebind
into the fresh document. ``_async`` cannot prevent that, so recovery says
when the runaway was still alive instead of implying it was fenced off.
"""
def __init__(
self,
call: str,
timeout: float,
watcher: Any,
*,
document_path: str | None = None,
document_id: int | None = None,
):
self.call = call
self.timeout = float(timeout)
self.watcher = watcher
self.document_path = document_path
self.document_id = document_id
self.stamp = time.monotonic()
self.outcome = "running"
self.detail = ""
self.released_by: str | None = None
self.result_lost = False
self.result_repr: str | None = None
def age(self) -> float:
return time.monotonic() - self.stamp
def thread_alive(self) -> bool:
return not self.watcher.finished
def capture_lost_result(self) -> None:
"""Record that the return value never reached the caller.
A late-completing ``entity_create_*`` leaves an entity in the document
whose handle nobody holds. That is not recoverable, so it is at least
reported rather than dropped.
"""
self.result_lost = True
try:
text = repr(self.watcher.result)
except Exception: # a __repr__ that raises must not break the report
text = f"<unreprable {type(self.watcher.result).__name__}>"
if len(text) > _MAX_RESULT_REPR:
text = text[: _MAX_RESULT_REPR - 3] + "..."
self.result_repr = text
def to_dict(self) -> dict:
return {
"call": self.call,
"timeout": self.timeout,
"age_seconds": round(self.age(), 3),
"thread_alive": self.thread_alive(),
"outcome": self.outcome,
"detail": self.detail,
"released_by": self.released_by,
"result_lost": self.result_lost,
"lost_return_value": self.result_repr,
"document_path": self.document_path,
}
def quarantine_refusal(record: QuarantineRecord) -> DocumentQuarantineError:
"""The refusal every later call on a quarantined document gets."""
thread = "still running" if record.thread_alive() else "has since stopped"
return DocumentQuarantineError(
record,
f"ezdxf document quarantined: '{record.call}' overran "
f"EZDXF_CALL_TIMEOUT={record.timeout:g}s {record.age():.1f}s ago and its worker "
f"thread ({thread}) cannot be cancelled, so it may still be mutating this "
"drawing. Every call on it is refused, reads included - a read of a container "
"another thread is appending to does not fail loudly, it never finishes. "
"Recover with drawing_open(path), drawing_new() or drawing_close(): those "
"rebind the document to a freshly constructed object the abandoned thread "
"cannot reach. Undo and rollback are not a way back - their snapshots may "
"themselves have been serialised mid-write. system_status() stays answerable "
"and reports the abandoned call.",
)
class DocumentQuarantineError(RuntimeError):
"""The open document is untrusted because a call was abandoned on it.
Carries **no** ``capability`` attribute, on purpose.
``server._batch_capability_of`` classifies refusals structurally — any
exception with a non-empty ``capability`` str plus a callable ``to_dict()``
is reported to the client as ``kind: "unsupported"`` and filed by
``cad_batch`` as a capability step row. Borrowing one here would tell the
client the ezdxf engine cannot draw a circle. The engine is fine; the
document is not. This needs its own kind, not a borrowed one.
"""
def __init__(self, record: QuarantineRecord, message: str):
super().__init__(message)
self.quarantine = record.to_dict()
def to_dict(self) -> dict:
return {
"ok": False,
"kind": "quarantined",
"error": str(self),
"quarantine": self.quarantine,
}