Repository navigation
Harden vector verification, audits, and cross-language conformance - #9
Conversation
The verifier cast any supplied vector to the pinned dtype before hashing, so every float64 value that rounds to the same float32 verified against an f32 pin: ~28 unverified bits per element in any float64-backed store (VP-1). It also raised on lone surrogates (VP-6) and naive KeyEntry datetimes (VP-11), accepted regex-shaped but impossible calendar dates (VP-9), padded or non-canonical base64url signatures (VP-12), float-valued v (VP-13), unbounded kid/model on the dict path (VP-14), empty vectors at sign time (VP-16), and offered no way to check model_hash (VP-10) or to bind the revocation window to a clock the key holder does not control (VP-5). Verifier.verify now refuses a vector wider than vec_dtype as SHAPE_MISMATCH, checks source before vector per spec section 5, takes expected_model_hash and trusted_time, and never raises on pin content. Signer.pin defaults vec_dtype to the input array's native dtype and rejects empty vectors. The parser rejects surrogates, enforces calendar validity, strict canonical base64url, integer v, and per-field byte caps. hash_files implements the model_hash convention. Regression tests: test_precision_widening (f64 low-bit channel is rejected), test_parser_hardening, test_trusted_time, test_model_hash, test_adversarial_perturbations, test_property_canonical (hypothesis).
The shipped audit treated a missing or empty pin field as a harmless 'unpinned' record and exited 0 (VP-2), never passed record, collection, tenant, or model expectations to the verifier so pin-shuffle replay passed (VP-3), stopped walking at the first malformed pin because the adapter generator was finalized by the exception (VP-7), and the Chroma HTTP adapter sent auth headers over plaintext with no guard (VP-15). Every adapter also coerced vectors to float32 before the verifier saw them, closing the audit's eyes to the VP-1 channel. Adapters now return the store's native precision (schema-derived for LanceDB and pgvector; declared single precision for Chroma, Qdrant and Pinecone, with the narrowing verified exact), yield error-carrying records instead of raising, and treat empty pin fields as parse errors. BaseAdapter gains get_verified/iter_verified read wrappers. The audit loop fails on unpinned records unless --allow-unpinned, prints per-record ids, enforces vectorpin.record_id against the record it sits on, and adds --collection-id/--tenant-id/--expected-model/ --expected-model-hash. A signed collection manifest (count + Merkle root) can be written at ingestion and checked with --manifest, and --sign-summary-key signs the JSON summary (verify-summary checks it). Private key files that are group/world readable print a warning.
… Rust port The Rust verifier narrowed an f64 slice to f32 before hashing, so bits below float32 precision were never checked against an f32 pin (VP-1). The ts parser accepted any digits, so 2026-03-00 reached u32 day-of- year arithmetic that underflows (a panic with overflow checks, a wrapped date without) before the signature was examined (VP-9). v was truncated with 'as u32' so 4294967298 parsed as 2, and extra: null was accepted (VP-13); kid/model had no byte cap on the from_value path (VP-14); model_hash could not be required (VP-10); and the revocation window had no verifier-trusted clock (VP-5). ed25519-dalek was built without its zeroize feature, so SigningKey was not wiped on drop (VP-16). Verifier::verify returns the new PrecisionWidening variant for an f64 slice against an f32 pin, checks source before vector per spec section 5, and takes expected_model_hash and trusted_time. Timestamps are calendar-validated at parse time and converted with checked i64 arithmetic; parse_ts_unix is exported for building key windows. v and vec_dim use checked conversions; extra: null is rejected; kid/model are byte-capped before normalization. The ed25519-dalek zeroize feature is enabled and proptest is added as a dev dependency. tests/audit_fixes.rs covers each finding (the f64 low-bit channel is asserted rejected), VectorSmuggle-style perturbations, and property tests over canonical bytes; cross_lang.rs consumes the extended fixture schema (native input dtype, model hash, collection/tenant ids, key windows, trusted time).
The TypeScript verifier narrowed a Float64Array (or a plain number[], which is a 64-bit representation) to f32 before hashing, leaving the discarded bits unverified against an f32 pin (VP-1). The signer defaulted to f32 for 64-bit input, accepted lone surrogates that TextEncoder then silently turned into U+FFFD (VP-6), tolerated base64 padding and non-canonical trailing bits in sig (VP-12), could not see that JSON.parse had collapsed 2.0 to 2 for v (VP-13), accepted extra: null, had no byte caps on kid/model (VP-14), used V8's Date.parse which rolls 2026-02-30 forward (VP-9), could not require model_hash (VP-10), had no verifier-trusted clock for the revocation window (VP-5), and depended on Node's Buffer despite the Deno/Bun claim (VP-18). Verifier.verify now refuses a 64-bit vector against an f32 pin as SHAPE_MISMATCH, checks source before vector per spec section 5, takes expectedModelHash and trustedTime, and never throws on pin content. Signer.pin defaults vecDtype to the input's native dtype and rejects lone surrogates everywhere. The parser enforces calendar validity, strict canonical base64url via a pure-JavaScript codec, minimal integer literals for v/vec_dim on the JSON path, per-field byte caps, and rejects extra: null. utf8ByteLength replaces Buffer.byteLength. test/audit-fixes.test.ts covers each finding (the f64 low-bit channel is asserted rejected), VectorSmuggle-style perturbations, and fast- check properties over canonical bytes and the base64url codec.
The shared fixtures only perturbed vectors in the pinned dtype, had no float edge cases, and lacked KEY_EXPIRED, SHAPE_MISMATCH, COLLECTION_MISMATCH and TENANT_MISMATCH entries, so none of the three ports' suites exercised the precision-widening boundary, calendar validation, surrogate rejection, strict base64url, or integer parsing. v2.json gains f32 and f64 IEEE-754 edge-case vectors (signed zeros, subnormals, max, 1 + ulp) and an f64 input pinned as f32 whose values are exact round-to-nearest-even ties, carried with vec_native_dtype so every port signs from the native input and verifies with the cast. negative_v2.json gains widened_f64_low_bit_perturbation and widened_f64_exact_values (SHAPE_MISMATCH), a truncated-vector SHAPE_MISMATCH, padded and non-canonical-trailing-bits sig, four calendar-invalid timestamps, a lone surrogate, v: 2.0, extra: null, kid/model over the field caps, model_hash mismatch/missing, collection and tenant mismatches, and two KEY_EXPIRED cases (one via trusted_time). The Python and TypeScript consumers read the new fields (Rust's was updated with its port); the generator self-checks every positive fixture's hash against the cast of its native input.
…phy bound ci.yml had python, rust, and fixture-drift jobs but never executed the TypeScript tests, so the 'consumed in all three test suites' claim was unenforced and a canonicalization regression in the TS port would have shipped unnoticed (VP-8). No job audited dependencies, and cryptography<46 blocked every release that fixes the advisories pip-audit reports against 45.x. Add a typescript job (npm ci, typecheck, test on Node 20), make the fixture job run the Python, Rust, and TypeScript cross-language tests after regenerating testvectors/, add a supply-chain job running cargo audit, npm audit --omit=dev, and pip-audit, and run Rust clippy/test with --all-features. Drop the cryptography upper bound (the Ed25519 and serialization APIs used are stable across majors and the cap only pinned users to vulnerable releases) and add hypothesis to the dev extra for the new property tests.
The README said 'any modification breaks the signature' while every shipped adapter and verifier cast wider vectors before hashing, listed a poisoned ingestion pipeline as in scope while also excluding the key holder it co-locates the signer with (VP-4), claimed 'selectively delete pins' was defended while the audit treated deleted pins as harmless, described a cron job as 'continuously verifies', and claimed Deno/Bun support, all-three-suites CI, and a Symbiont runtime integration that did not hold (VP-18). The spec said 14-byte tag, required escaping U+007F, and ordered source before vector while the code did the reverse (VP-17); section 7 claimed revocation immediately invalidates post-compromise forgeries (VP-5); the troubleshooting guide documented cast-before-hash as intended; the CHANGELOG claimed the audit continued past a malformed pin (VP-7). State that the protected interval begins at signing and that signing should run as a separate service; add the precision rule to spec section 3.2, calendar validity and integer-literal rules to 4.1, field caps to 4.3, the never-raise contract and model_hash expectation to 5, the trusted-time limitation to 7 and 9, and the model_hash computation procedure. Document fail-closed audits, manifests, signed summaries, the read wrappers, the quantization boundary, and 'pin under the dtype your store persists'. Correct the stale claims, and add an Unreleased CHANGELOG section listing every fix by finding ID.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97b4df90e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| elif arr.dtype.itemsize > target.itemsize: | ||
| narrowed = arr.astype(target) | ||
| if not np.array_equal(narrowed.astype(arr.dtype), arr): | ||
| raise ValueError( |
There was a problem hiding this comment.
When an f32-backed REST API serializes a stored value using its shortest JSON representation (for example, 0.1), the client parses it as the f64 value 0.1, which is not exactly equal to the widened f32 value even though narrowing recovers the original stored value. This exact-equality check therefore raises for ordinary Qdrant REST responses and aborts the audit; once native_dtype is supplied by the adapter, values that round-trip through that dtype need to be accepted rather than required to already be its exact widened representation.
Useful? React with 👍 / 👎.
| if write_manifest is not None: | ||
| path, signer = write_manifest | ||
| built = build_manifest(signer, collection_id or label_value, entries) | ||
| path.write_text(built.to_json() + "\n") |
There was a problem hiding this comment.
When any record fails verification or the adapter iterator aborts partway through, this block still signs and writes a manifest for the invalid or partial entries list. Because the manifest is intended to establish the trusted post-ingestion baseline, publishing that file can bless an incomplete collection despite the nonzero exit status; only write it after a complete, successful walk.
Useful? React with 👍 / 👎.
| MAX_KID_BYTES = 256 | ||
| MAX_MODEL_BYTES = 1024 |
There was a problem hiding this comment.
The new 256-byte kid and 1024-byte model limits are enforced only by parsers. The Python, TypeScript, and Rust signers still accept larger values and return signed pins, so a pin produced by the library cannot be parsed back by the same library—for example, Signer.generate(key_id="k" * 257).pin(...) succeeds but Pin.from_json(pin.to_json()) fails. Mirror these limits in signer construction and pin creation.
Useful? React with 👍 / 👎.
| if (!isValidCalendarTs(ts)) { | ||
| throw new Error('pin.ts is not a valid calendar date-time'); |
There was a problem hiding this comment.
The new calendar check is not mirrored by the TypeScript or Rust signing paths. A TypeScript caller can pass timestamp: "2026-02-30T00:00:00Z" through formatTimestamp's regex fast path, and Rust PinOptions.timestamp likewise performs only string-safety checks; both produce signed pins that their newly hardened parsers immediately reject. Validate explicit timestamps with the same calendar predicate before signing.
Useful? React with 👍 / 👎.
| try: | ||
| datetime.strptime(ts, "%Y-%m-%dT%H:%M:%SZ") | ||
| except ValueError: | ||
| raise ValueError(f"{field_name} is not a valid calendar date-time") from None |
There was a problem hiding this comment.
Python's datetime.strptime rejects 0000-01-01T00:00:00Z, while the new Rust parse_ts explicitly treats that timestamp as valid and TypeScript's isValidCalendarTs plus Date.parse also accepts it. A pin with this otherwise pattern-conforming timestamp can therefore be signed and verified by those ports but cannot be parsed by Python, breaking the advertised cross-language wire contract; either reject year zero everywhere or validate it without Python's datetime year restriction.
Useful? React with 👍 / 👎.
1 participant
Vector verification rejects precision widening, validates attestation dates, binds the model hash and record identity, and fails audits closed. Python adapters and CLI behavior follow the hardened contract, with matching Rust and TypeScript verification changes.
Shared positive and negative fixtures cover float edge cases and malformed attestations. CI now exercises all three implementations, fixture regeneration, and dependency checks; documentation states the resulting guarantees and deployment limits.
Validation: All seven CI jobs passed: Python 3.11–3.13, Rust, TypeScript, cross-language conformance, and dependency audits. No package release or tag is part of this PR.