v0.6.3 security, artifact integrity and release-state hardening - #45
Merged
Conversation
`bridge doctor` imported reward/reward_or_verifier_scaffold.py with exec_module, so any top-level statement in an untrusted bundle ran as soon as a user asked for a diagnosis - while the report called it a "side-effect-free import". Replace it with an AST walk that verifies the compute_score interface without evaluating anything, and gate the old behaviour behind --trust-and-import-reward-code, which labels the run as untrusted code execution rather than isolation. The safetensors check parsed the header and stopped at the first tensor key, so a file declaring a 4x4 F32 tensor with no payload passed while the official reader rejects it. Validate dtype/shape byte arithmetic, offset ordering, contiguity and full coverage of the data segment, then materialize through the official reader. Report not_present, header_only, payload_structure_validated or tensor_materialization_validated, and add --require-adapter-payload for callers that must not accept a header.
import-verl accepted --out pointing at its own source config. With --overwrite a successful import replaced the user's input, and a rejected import deleted it while publishing only the rejection report. Reject any overlap between an input and the intended output family before the transaction is created, using samefile() where both paths exist and normalized case-folded comparison otherwise, so symlink, hard-link, relative and Windows case aliases are all covered. There is no in-place mode, so --overwrite never authorizes destroying an input. A row could carry miniVERL extension data in miniverl_extensions, the conversion sidecar and extra_info.miniverl at once; the converter picked one and dropped the rest. Collect all three, accept canonical-equal duplicates as a recorded deduplication, and fail closed when they differ, naming the row and the source locations but never the values, which may contain teacher targets. Conversion also published the accepted rows of a partially invalid dataset while the module described itself as lossless. Make it complete-or-nothing by default and add --allow-rejected-rows for an explicit partial run, whose report carries complete_dataset_conversion: false, lossless_for_accepted_rows: true and the output-to-source row index map.
The "bounded" dataset scan called pq.read_table() and table.to_pylist() and only then applied max_rows/max_bytes, so the bound described what was reported rather than what was read. The schema check materialized every row to learn its column names. Either would stall or exhaust memory on a verl-scale dataset. Read schema names, row counts and row-group counts from the Parquet footer, and stream the scan through ParquetFile.iter_batches one row group at a time, restricted to columns whose Arrow type can contain a string, stopping the moment a bound is reached. Files past the bound still contribute their footer row count and are never decoded. The report now carries files_total, files_inspected, row_groups_read, rows_scanned, rows_total, bytes_scanned, the batch size and the configured bounds, so a sampled result is visibly sampled. An instrumented test records the row_groups argument of every iter_batches call and asserts it is exactly [0] once the row bound is two rows into an eight-group file.
Publishing v0.6.2 and advancing main to 0.6.3.dev0 left the repository telling four different stories at once: both READMEs said stable was v0.6.1, the docs selector offered "Stable 0.6.1 / Development 0.6.2.dev0", and the quality record paired release 0.6.2 with a floor measured "at v0.6.1" - which a test had pinned in place. Add release-state.yaml as the single source and scripts/release_state.py to project it onto the package __version__, both READMEs, PYPI.md, the docs channel selector, CITATION.cff, the changelog comparison link, the release checklist section, the PROJECT_STATE header and the quality record. --check is the gate and reported all ten disagreements; --write regenerates the mechanical ones. Wire the gate into ci.yml and release.yml. A test drives the full post-release transition - release 0.6.3, stable 0.6.3, main 0.6.4.dev0 - and asserts every claim follows without manual editing, while the prose sections a person must write are still reported as missing rather than silently passing. Update CITATION.cff to describe the software as it now is: an auditable single-GPU alignment and distillation runtime with comparable SFT, DPO, KD and OPD arms, shared-backbone role switching, padded updates, mechanism studies published with their negative results, and a fail-closed verl artifact bridge that explicitly does not launch distributed jobs or claim PPO parity.
v0.6.2 shipped consumer-runtime-v1-pareto, cost-quality-pareto, fresh-vs-frozen and recovery-success on an explicit mobile-readability exemption list. The exemption was honest - it printed on every run - but the pages are public and those figures were still unreadable at 390px, where a 1120px canvas scaled into the 358px content column renders 16px text at about 5px. Generate a dedicated 390px companion for each. The three RecoveryBench charts become vertical card lists where every bar carries its series name and its exact value as text, so nothing is encoded by colour alone. The consumer-runtime scatterplot, which cannot survive a narrow column at all, becomes an accessible table of throughput and reserved memory for both runtimes at every batch size, with the knee stated in words. Each page selects them with a <picture> media query, so the desktop source stays inactive rather than being scaled. The desktop figures are byte-identical: their pinned SHA-256 values are unchanged. Only the generators were extended. check_docs_visual.py now enforces the 11px floor for every figure at every viewport with no per-figure opt-out, and the run reports 28 rendered SVG instances across the four viewports with zero skips. The first attempt at the mobile titles failed the gate's own overlap check at 390px, which is the check working.
The bridge output transaction provides per-stem locking, same-filesystem staging, collision checks and in-process rollback. It does not survive kill -9, a kernel panic or power loss between two renames, so calling it "atomically publish one output family" claimed more than the code does. State the guarantee as transactional publication with in-process rollback, say plainly what a crash can still leave behind, and name the versioned-directory design that would be required to fix it rather than implying it exists. Document the rest of the release in both READMEs and the bridge guide: reward code is statically inspected and never executed by default, the dynamic import is explicitly unsafe, safetensors and tokenizer results report how far verification got, conversion is complete-or-nothing unless --allow-rejected-rows is given, extension conflicts fail closed without printing values, the privacy scan's bounds are enforced while reading and its counters are published, and an input file can never also be an output.
DaoyuanLi2816
marked this pull request as ready for review
August 5, 2026 08:13
The torch-free environment installs safetensors but not numpy, and safe_open(framework="np") needs numpy. The resulting ModuleNotFoundError was caught by the generic except and reported as "the official safetensors reader rejected adapter_model.safetensors", which failed the model check, the bundle verdict and nine tests against files that were structurally fine. A dependency gap is not evidence about the file. _materialize now returns materialized, unavailable or rejected: a missing safetensors package, a missing numpy, or an ImportError surfacing from inside the reader are all unavailable, and only a real reader error is a rejection. On unavailable the level stays at payload_structure_validated with official_reader_status: dependency_missing and the status stays passing, because the structural pass does not depend on the official reader. The tests no longer assume the reader can run, and the exact CI condition - safetensors present, numpy absent - is now a regression test.
A release tree *is* the release, but release-state.yaml could only express "stable X, development X.devN". That is why v0.6.2's own tag shipped a docs selector still advertising "Stable 0.6.1 / Development 0.6.2.dev0": there was no way to say "this tree is 0.6.2". Add `phase: development | release`. In the release phase development.version must equal the version being published and release_commit may be the literal `pending`, because a release commit cannot name its own merge SHA; the post-release sync fills it in. Both phases are covered by tests. The [bridge] extra installed pyarrow but not numpy, and the official safetensors reader needs numpy for framework="np". So in the very extra that exists for bridge work, `bridge doctor --require-adapter-payload` could never be satisfied. Declare numpy there. Verified in a clean environment: the extra now reaches tensor_materialization_validated with torch still absent. Also stop tracking the local browser-screenshot directory and the takeover archive, and record the safetensors dependency-gap defect in PROJECT_STATE and the changelog.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A narrowly scoped security, artifact-integrity, data-conversion and
release-state release. No benchmark was rerun and no new scientific or
distributed-runtime claim is added. All ten frozen result artifacts and the
pinned bridge-smoke record are byte-identical.
Security
bridge doctorexecuted arbitrary code from the inspected bundle. Thereward check imported
reward/reward_or_verifier_scaffold.pywithexec_module, so any top-level statement ran with the user's privileges assoon as they asked for a diagnosis — while the report described the result as
side-effect-free import; scaffold intentionally not executed. Reproducedfirst: a scaffold whose top level wrote
PWNED.txtcreated the marker and wasreported
status: ok.src/miniverl/bridge/reward_static.pyreplaces it with an AST walk that neverimports. Levels are
not_present,syntax_valid,interface_statically_verified,trusted_dynamic_import_verified; the defaultpath stops at the third. Top-level calls, non-literal assignments, decorators,
class-body statements and call-valued default arguments are all rejected,
because each runs at import time.
--trust-and-import-reward-coderestores theold behaviour for self-produced bundles, warns first, reports
untrusted_code_executed: true, and does not claim to be a sandbox.A truncated adapter could pass. A header declaring a 4x4 F32 tensor with
zero payload bytes returned
(True, '1 tensor header(s)')while the officialreader rejected it with
file not fully covered.src/miniverl/bridge/safetensors_check.pyvalidates dtype/shape bytearithmetic, offset ordering, contiguity and full coverage, then materializes
through the official reader. Levels:
not_present,header_only,payload_structure_validated,tensor_materialization_validated, with--require-adapter-payloadfor strict callers.import-verlcould destroy its own source. With--outpointing at thesource config, a successful import overwrote it and a rejected import
deleted it while publishing the rejection report.
reject_source_output_aliasruns before any transaction and covers exact, relative, symlink, hard-link and
Windows case aliases for both
import-verlandconvert-dataset. There is noin-place mode;
--overwritereplaces a previous output family and neverauthorizes destroying an input.
Correctness
miniverl_extensions, the conversion sidecar andextra_info.miniverlwas silently reconciled by preference. Equal content nowdeduplicates and is recorded; disagreeing content fails closed, naming the row
and the locations but never the values, which may carry teacher targets.
the module called itself lossless. It is complete-or-nothing by default;
--allow-rejected-rowsopts into a partial run whose report sayscomplete_dataset_conversion: falseand keeps the source row index map.read_table().to_pylist()before applyingits bounds, and the schema check read every row to learn column names. Both
now use footer metadata and per-row-group
iter_batches. An instrumented testrecords the
row_groupsargument of every call and asserts it is exactly[0]when the bound is two rows into an eight-group file.Release state and presentation
release-state.yamlis now the only source for stable/development versionclaims, projected onto both READMEs,
PYPI.md, the docs selector,CITATION.cff, the changelog link, the checklist,PROJECT_STATE.mdand thequality record.
scripts/release_state.py --checkreported all tendisagreements that existed at branch point and is wired into CI. A test drives
the full post-release transition and still demands the prose sections a person
must write.
390px layouts; the exemption list is empty and the floor is enforced for every
figure at every viewport. Desktop figure bytes are unchanged.
CITATION.cffdescribes the actual scope of the software.atomicity is explicitly not claimed.
Verification
pytest -m "not gpu and not network"pytest -m gpupytest -m networkmkdocs build --strictpython -m build+twine checkverl-bridge-smoke.jsonbyte-identicalThe one skip is symlink creation, which needs privileges on Windows; the
hard-link and case-alias cases cover the same guard there.
Issue #39 remains open and out of scope.