Audit fixes: 32-bit sentinel bug, repo identity, pyramid lifecycle, release gate, FFI hardening - #7
Merged
Conversation
The size-returning kernel wrappers compared the Rust error sentinel against 2**64-1. On 32-bit targets (armv7 / win32 / wasm32 — all shipped wheels) c_size_t is 32 bits, so the sentinel arrived as 2**32-1, never matched, and error returns — including panics converted by the ffi_guard shield — were sliced as valid data from an uninitialized buffer. Derive the sentinel from ctypes.c_size_t once at module level, use it at all five call sites, and replace the -O-strippable zone_maps assert with a real check. The regression test simulates the 32-bit ABI and fails on the old comparison.
SECURITY.md routed private vulnerability reports to a stale personal repo; CONTRIBUTING.md cloned it; the CodSpeed badge linked it; CLAUDE.md carried the old project name; PyPI metadata had no [project.urls] at all. Point everything at github.com/reflex-dev/reviz, and teach check_claim_guardrails.py to reject stale repo identities in public docs (SECURITY.md and CONTRIBUTING.md are now scanned too). Also drop an internal dev-sandbox reference from docs/benchmark.md.
_ensure_pyramid stashed a raw registry handle on the trace, and the only free was on the streaming-append path — a Figure discarded any other way (the notebook cell-re-run pattern) leaked the pyramid in the process-lifetime Rust registry. Register a weakref.finalize on the Trace so GC frees the handle, route the append path through the same helper so an explicit free disarms the finalizer, and add a pyramid_bytes line to Figure.memory_report() (§27: if it isn't in the report it isn't real).
The '10M-point line' cell allocated n = 10_000_00000 — one billion points, 8-16 GB of arrays on a laptop — and the '30M-point scatter' cell allocated 300M with a title saying 300M under a 30M heading. Align code, headings, and chart titles at the advertised 10M/30M.
release.yml published whatever version sat in pyproject.toml under any v* tag; nothing checked the tag against the package version or required a dated CHANGELOG entry. Add scripts/check_release_version.py as the first step of the publish job on tag pushes, needle it in verify_ci_workflow.py, and test it. Also fix the stale 'falls back to NumPy' sdist comment in release.yml (there is no fallback) and restore a truncated comment in ci.yml.
fc_rasterize guarded its size product with checked_mul but fc_bin_2d, fc_bin_2d_indices, fc_m4_indices, and fc_pyramid_compose built slices from bare w*h / n_buckets*4, which wrap silently in release builds into shorter-than-expected slices. Guard all four the same way. Index-emitting entry points (m4, range, bin_2d_indices) now also reject len > u32::MAX, where emitted u32 row ids would wrap into valid-looking wrong rows. All guards fire before any pointer is dereferenced; error sentinels match each entry point's existing convention. No ABI signature changes.
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
1 participant
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.
Fixes the P0/P1 findings from a full architecture audit. Six commits, each self-contained with tests.
Correctness
usize::MAXsentinel detection (_native.py): the five size-returning kernel wrappers compared the Rust error sentinel against2**64-1. On 32-bit targets (armv7 / win32 / wasm32 — all shipped wheels)c_size_tis 32 bits, so error returns — including panics converted by theffi_guardshield — were sliced as valid data from an uninitialized buffer. Now derived fromctypes.c_size_t(-1).value; the regression test simulates the 32-bit ABI and fails on the old comparison. The-O-strippableassertinzone_mapsis now a real check.src/lib.rs):checked_mulon grid size products infc_bin_2d,fc_bin_2d_indices,fc_m4_indices,fc_pyramid_compose(matching whatfc_rasterizealready did), and alen > u32::MAXceiling in the index-emitting entry points where u32 row ids would wrap into valid-looking wrong rows. No ABI signature changes.Lifecycle
_ensure_pyramidregisters aweakref.finalizeon the Trace (per the rust-engine design doc §3.3), the append path frees through a helper that disarms the finalizer, andmemory_report()gains apyramid_bytesline (§27: if it isn't in the report it isn't real). Previously a Figure discarded by a notebook cell re-run leaked its pyramid in the process-lifetime registry.Release safety
scripts/check_release_version.pyruns first in the publish job on tag pushes — the tag must equalv<pyproject version>and CHANGELOG.md must carry a dated entry for it. Needled intoverify_ci_workflow.pyand tested. Note: cutting the next release now requires dating the## [0.1.0]changelog heading, by design.Identity / docs
Alek99/charts-exp); all now point atreflex-dev/reviz,pyproject.tomlgains[project.urls], andcheck_claim_guardrails.pynow rejects stale repo identities (SECURITY.md/CONTRIBUTING.md are scanned too).10_000_00000= 1 billion points (8–16 GB); the "30M scatter" allocated 300M. Aligned code, headings, and titles.Verification
Full pytest suite (904 passed),
cargo test(61 passed),cargo clippy -D warnings,ruff check+format --check,ty check,check_public_api,check_python_floor,check_claim_guardrails,verify_ci_workflow,node js/build.mjs --check, andscripts/abi_smoke.py(70/70) all pass locally.🤖 Generated with Claude Code