Skip to content

FIX: incremental run listing off the event loop, model uploads refused, runs in flight capped per workstation - #98

Merged
Jules-GP merged 2 commits into
DCBIA-OrthoLab:mainfrom
Jules-GP:audit/security-performance
Oct 30, 2026
Merged

Jules-GP merged 2 commits into
DCBIA-OrthoLab:mainfrom
Jules-GP:audit/security-performance

Conversation

@Jules-GP

Copy link
Copy Markdown
Collaborator

Why

A security and performance audit of the stack, in three rounds (findings, then the fix diff twice). This is what it found in the server. Numbers are measured on a local instance with 500 run directories, about 335 events per run.

Performance

Finding Before After
runs.active() re-read and re-verified every event of every run at each call; /status, /admin-panel.json (twice), updates.json and the update agent call it, the pages every 2 s panel 2.5 s per call; live with one admin tab: panel median ~100 s, 4 of 5 /status timed out, which the update agent's wait_idle reads as "server down" and restarts it with runs in flight panel 9.8 ms, /status 10 ms, no timeout
Staged inputs deleted (rmtree/remove) on the event loop worst /health 200 ms during a 2 GB discard 4 ms
Admin and status pages fired the next poll whether or not the last one answered 11 requests stacked in 20 s on a slow answer at most 1 or 2
disk() walked TEMP_DIR and DATA_DIR at every panel poll ~25k stat per poll 1 walk per 30 s
runs.finish() parsed the whole events file on the loop 0.5 s for a chatty tool's 100k lines off the loop
One nvidia-smi per /status and per panel poll 3 execs for 3 calls 1 per trace interval
Upload part copied once more than needed 2.10x the part in memory 1.10x

The listing is now incremental: one reader per run, kept between listings and keyed on inode, size and mtime, reads only what a run appended, still verifies the signature of every new line, and stops at the terminal event as a whole read does (a 12-seed random-append test checks it gives the same answer as a whole read). Also: the event stream sends a : ping every 15 s of silence (the client's read timeout is 30 s, so a quiet stretch was a reconnect and a full replay); runs a previous server process left pending or running are failed at startup with "The server restarted during this run." (paused runs untouched); container logs are bounded (json-file, 5 x 20 MB).

Security

  • Uploads to a model argument are refused (422). deployment.toml opts 14 model arguments out of the name-only convention with "none", which also made them uploadable files; AREG IOS then unpickled such an upload (fixed in the tool, SADT-VISOR#99). The server now refuses an upload to model / *_model in both staging paths, and hides those arguments from the published schema so a panel offers no upload row. *_reference arguments stay uploadable: none feeds a pickle-based loader.
  • A cap on each workstation's runs in flight, MAX_RUNS_PER_CLIENT (default 10, 0 = none), 429 before anything is staged. Pending and waiting-for-the-card runs count, paused ones do not; a run sent without X-Run-Id holds a slot while it runs. The count, the registration and the slot claim are one locked step in a worker thread, so two requests cannot both pass, and a cancelled request keeps no slot. A benchmark battery an operator launched is exempt through a per-battery random key that never reaches a tool's environment. The cap reads the client address uvicorn derives, so behind a proxy it depends on FORWARDED_ALLOW_IPS and the proxy overwriting X-Forwarded-For.
  • ToolExecutionError was raised in main.py without being imported: a detached run whose result could not be stored failed with a NameError.

Tests

New tests count (calls, stats, fetches, bytes), not time; each failed before its fix. A test that a cancelled /run keeps no slot fails when _off_loop's shield is removed. Server suite: 1554 passed; the known test_dev_mounts failure is unchanged, and the update-agent suite passes 57/57 when rerun. bandit and zizmor clean.

Deferred: AREG CBCT copies the oriented scans out of supervisor scratch instead of moving them (unmeasured).

…l weights and cap each workstation's runs in flight
Copilot AI balanced review requested due to automatic review settings October 30, 2026 02:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

Comment thread server/tests/test_page_polling.py Fixed
@Jules-GP
Jules-GP merged commit c83562f into DCBIA-OrthoLab:main Oct 30, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants