Add server execution and orchestration

[kiddyboots216]

Summary

Add server execution and orchestration: ModelRunner, runner adapters and checkpoint management, API/backend/orchestrator flows, packing, ZORL, launcher behavior, the OPD pipeline, and final server-facing integration hooks.

Validation

• Server execution and orchestration imports passed.
• Focused suite: 163 passed.
• Review follow-up session-state regression selection: 12 passed.
• The authoritative final Foundation publication-security suite passed: 73/73.
• Repository public-reference guard, all-files pre-commit, and git diff --check passed.

Stack

This is 9/9 and the replacement Foundation tail. Existing Numerical PR #22 will be retargeted here after the replacement stack is green.

[broly-code-security-scanner]

Broly Security Scan

Note

Summary

1 actionable finding(s) in this PR

• 🟡 1 medium

All actionable items are in the table below.

No finding is at or above high, so this check is not blocking. The findings above are still tracked and reported.

Severity	Scanner	Issue	Location	Dismiss	Verdict
🟡 MEDIUM	SAST	Unsafe deserialization via torch.load of attacker-controlled file paths	src/xorl/server/runner/model_runner.py:2602	d15	🔺 TRUE_POSITIVE · Confidence: HIGH

Dismiss false positives

Tick a box to dismiss the finding; untick it to bring the finding back. That is the same as replying /broly dismiss d1 and /broly undismiss d1. To record why it is a false positive, reply with /broly dismiss d1: your reason instead — Broly reuses those reasons to triage similar findings across the org.

• d15 · 🟡 MEDIUM   · src/xorl/server/runner/model_runner.py:2602 · Unsafe deserialization via torch.load of attacker-controlled file paths

Note

Re-scan this PR anytime with /broly scan — useful after /broly undismiss, or to refresh findings without a new push.

✅ Recorded false-positive dismissals (3)

• d6 · src/xorl/server/runner/runner_dispatcher.py:1143 — @kiddyboots216: Filesystem R3 references are not accepted from the public API. The orchestrator creates them from its operator-configured r3_payload_dir after converting API routing lists, adds a nonce to the sanitized request ID, and broadcasts them internally. The runner then requires an absolute non-symlink manifest.json, a fixed format/version, fixed child directories and filenames, validated shapes/dtypes/sizes, and rejects symlinked children.. Takes effect on the next Broly scan (or /broly scan).
• d7 · src/xorl/server/orchestrator/request_processor.py:255 — @kiddyboots216: safe_request_id replaces every character except alphanumeric, dot, underscore, and hyphen before the value is used, falls back to the literal request when empty, and appends a fresh hexadecimal UUID suffix. The resulting component cannot contain a path separator or dot-dot component.. Takes effect on the next Broly scan (or /broly scan).
• d8 · src/xorl/server/runner/model_runner.py:2603 — @kiddyboots216: The path is first passed through resolve_diagnostic_input, which requires an operator-configured XORL_DIAGNOSTIC_INPUT_ROOT, canonical containment, no symlinks, a regular file, no group/world write bits, and an 8 GiB size cap. The pinned PyTorch 2.12 runtime then loads with weights_only=True and the result is type-checked as a dict of tensors.. Takes effect on the next Broly scan (or /broly scan).

Reverse any of these with /broly undismiss <key> (e.g. /broly undismiss d2) — the finding resurfaces on the next scan (or /broly scan).

Broly — SAST (zai-org/GLM-5.2) · Secrets · SCA · IaC · GH Actions · Base Images · Supply Chain Threats · Exploit Chains · Adversarial Verification

We're continuously improving Broly's accuracy and finding quality — your feedback is valuable. False positives, missed findings, bugs, and feature requests all welcome.

 

[kiddyboots216]

/broly dismiss d6: Filesystem R3 references are not accepted from the public API. The orchestrator creates them from its operator-configured r3_payload_dir after converting API routing lists, adds a nonce to the sanitized request ID, and broadcasts them internally. The runner then requires an absolute non-symlink manifest.json, a fixed format/version, fixed child directories and filenames, validated shapes/dtypes/sizes, and rejects symlinked children.

[kiddyboots216]

/broly scan

[kiddyboots216]

/broly dismiss d7: _safe_request_id replaces every character except alphanumeric, dot, underscore, and hyphen before the value is used, falls back to the literal request when empty, and appends a fresh hexadecimal UUID suffix. The resulting component cannot contain a path separator or dot-dot component.

[kiddyboots216]

/broly scan

[kiddyboots216]

/broly dismiss d8: The path is first passed through resolve_diagnostic_input, which requires an operator-configured XORL_DIAGNOSTIC_INPUT_ROOT, canonical containment, no symlinks, a regular file, no group/world write bits, and an 8 GiB size cap. The pinned PyTorch 2.12 runtime then loads with weights_only=True and the result is type-checked as a dict of tensors.

[kiddyboots216]

/broly scan

[qywu]

Reviewed: description is clear, CI passing, no suspicious file changes. LGTM.

[qywu]

Deep review

I read the full diff (~20.4k lines / 62 files) — ModelRunner, the runner dispatcher, checkpoint manager, adapter manager, orchestrator/packing/request_processor, the launcher, zorl.py, and the OPD pipeline script — plus fetched several full source files for context. Overall this is a well-hardened PR: pickle.dumps/loads was removed from the checkpoint shard-gather path in favor of json (checkpoint/manager.py), the launcher's subprocess.Popen calls stay list-form with no shell=True and gained explicit argv sanitization (_validated_subprocess_value), and most new save/load paths were retrofitted with validate_identifier/resolve_server_artifact path-traversal guards. Tests exist for the major new surfaces (packing strategies, ZORL, adapter manager, runner dispatcher).

That said, I found one concrete orchestration bug and two smaller correctness/consistency issues.

1. start_zorl_generation can permanently wedge a ZORL session on ordinary bad input

src/xorl/server/runner/model_runner.py:759-789

plan = zorl_state.begin_generation(num_pairs=num_pairs)          # line 772 — mutates zorl_state.active_generation
...
materialization_plan = normalize_zorl_materialization(materialization, num_pairs=global_num_pairs)  # line 774 — can raise
...
export_dir = self._zorl_generation_export_dir(plan.generation_id)
session_spec = self.get_lora_session_spec(model_id)

try:                                                               # line 788 — cleanup starts HERE
    ...
except Exception:
    if zorl_state.active_generation is not None and zorl_state.active_generation.generation_id == plan.generation_id:
        zorl_state.abort_generation(plan.generation_id)
    ...
    raise

ZORLSessionState.begin_generation() (src/xorl/server/zorl.py:277-345) sets self.active_generation and then raises ValueError on any subsequent call while a generation is active (zorl.py:280-283). But normalize_zorl_materialization — which validates the client-supplied materialization field of POST /api/v1/.../start_zorl_generation (ZORLStartGenerationRequest.materialization, api_server/api_types.py — Pydantic only types it as Optional[ZORLGenerationMaterialization], it doesn't range-check num_shards/shard_index/pair_start/pair_end) — is called after begin_generation() but before the try: block that does the cleanup/abort on failure.

So a client request with e.g. {"materialization": {"mode": "pair_shard", "num_shards": 0}} (or shard_index out of range, or pair_start > pair_end) raises inside normalize_zorl_materialization (zorl.py:416-417 / 428-433), the exception propagates out of start_zorl_generation uncaught, and zorl_state.active_generation is left set to the now-orphaned plan. The caller never received a generation_id (the response never got that far), so they have no way to call abort_zorl_generation to clear it. Every subsequent start_zorl_generation call for that model_id then fails with "ZORL generation ... is still active" until the process is restarted or someone reverse-engineers the internal family_id-g{generation:06d} id from logs.

Fix: move the try: up to wrap begin_generation() through the materialization/candidate-id validation (or validate materialization before calling begin_generation), so any failure in setup aborts the just-created generation.

2. Backend default for pause_mode diverges from the documented/API default

src/xorl/server/backend/remote.py:371 vs src/xorl/server/api_server/api_types.py:1218

RemoteBackend.sync_inference_weights()'s own default changed from pause_mode="in_place" to pause_mode="retract", but SyncInferenceWeightsRequest.pause_mode (the actual API-facing Pydantic default) is unchanged at "in_place", and backend/base.py/backend/dummy.py were not updated to match. In the production request path this is masked because request_processor.py always forwards p.pause_mode explicitly, but any direct caller of RemoteBackend.sync_inference_weights() that omits pause_mode (tests, scripts, future SDK use) now silently gets different pause semantics than the documented default, and than DummyBackend/Backend present. Worth aligning the default (or dropping it and making the argument required) so the three layers agree.

3. Dead/ineffective validation ordering in LoRAAdapterManager.capture_gradients

src/xorl/server/runner/adapters/manager.py:779-781

if model_id not in self.adapters:
    raise KeyError(f"Adapter for model_id={model_id} not registered")
model_id = validate_identifier(model_id, name="model_id")

validate_identifier is called after the membership check, so it only ever runs on a model_id that's already a valid registered key — it can never reject anything here (contrast with save_adapter_state/load_adapter_state/register_adapter in the same file, where validation correctly precedes use). Not exploitable on its own, but it's inconsistent with the hardening pattern applied everywhere else in this PR and gives a false impression that this path is validated.

Verdict

Item 1 is a real, client-triggerable orchestration bug (self-inflicted session lockout from routine bad input), so I'm requesting changes. Items 2-3 are minor and worth fixing but non-blocking.

[qywu]

Approved per maintainer direction, superseding the change-request review above. See prior review comment for the technical issues found; these are not resolved in the diff, tracking as follow-up.