Skip to content

v0.4.2 — Code-review hardening

Latest

Choose a tag to compare

@queelius queelius released this 21 May 16:28
· 15 commits to master since this release

Patch release hardening pagevault against issues found in a multi-lens code review (code review + simplifier + design review) of the v0.4.0/v0.4.1 architecture overhaul. No wire-format breaks for existing files; two small CLI behavior changes are noted under Upgrade.

Security

🔴 Forged postMessage into the site-renderer iframe

The fetch shim injected into srcdoc site iframes (nav_injector.js) listened for pagevault-fetch-response messages without verifying e.source. Any frame holding a handle to the iframe's contentWindow could forge a response and steer the shim's fetch() to an attacker-controlled URL. The parent side already verified the source; the iframe side now does too (e.source !== window.parent is rejected).

🟡 Toolbar built without innerHTML

createToolbar / renderDownloadView concatenated a blob: URL into an innerHTML template. Safe today (blob URLs are UUID-shaped) but fragile. Both now build their element trees with createElement + property assignment; textContent cannot be bypassed by a future escape-skipping path.

🟡 Envelope chunk_count validation

decrypt_v4 trusted total_size for short-circuit decisions: a tampered envelope claiming total_size: 0, chunk_count: 5 decrypted silently to empty bytes, ignoring the phantom chunks. decrypt_v4 now validates chunk_count against the actual chunk list up front, surfacing tampering (and transport truncation) with a clear error before any GCM work.

Fixes

🔴 renderFile left the page in a broken state on viewer failure

data-decrypted="true" was set before the viewer's async render. If a viewer threw (bad markdown, plugin bug), the page kept the attribute set with the password prompt still in the DOM. The attribute is now set only after a successful render, and the render dispatch is wrapped in try/catch with a visible error UI.

🟡 lock_html parser inconsistency

unlock_html hard-coded BeautifulSoup("html.parser") for the decrypted fragment while every other parse used the lxml-preferred _parse_html(). The two parsers disagree on table/tbody insertion, <select> bare text, and <a> nesting, so a lock → unlock round-trip could drift. Now consistent.

🟡 lock no longer re-locks its own output

pagevault lock . -r with the default _locked/ output would rglob into the output directory and re-lock already-locked files in place. Recursive scans now skip files under the output directory (with a notice); explicitly pointing a source path inside the output directory is a hard UsageError.

🟡 Fail-loud runtime injection

If input HTML had no <html> root, _inject_runtime silently skipped injection while lock_html still emitted the encrypted payload — a file that could never be unlocked in a browser. It now raises a clear PagevaultError.

🟢 inspect warns on mixed envelopes

A pathological file containing both a wrap (data-pv-chunked) envelope and region (data-pv-v4) envelopes previously showed only the wrap info. inspect now prints a warning listing the hidden region count.

Improvements

  • Byte-level region padding. Region encryption switched from string-level pad_content (UTF-8 encode → append NULs → lossy re-decode → rstrip on unlock) to byte-level padding with meta.size. Brings regions in line with wraps; old envelopes without meta.size still decrypt via the legacy fallback.
  • Property-based test suite stabilized (Hypothesis deadline disabled for crypto-bound tests — PBKDF2 cost is uniform per input, so the deadline measured CPU noise, not a property of the inputs).
  • 17 new tests (706 → 723); a few simplifier cleanups (json import hoist, dead v0.3 branch removed, single-pass v4 region scan).

Upgrade

pip install --upgrade pagevault

Two CLI behavior changes:

  • -p with a multi-user config is now an error. Previously pagevault lock -p PASSWORD against a users: config silently dropped every configured user from the output file. It now raises a UsageError; use -u USERNAME -p PASSWORD for an ad-hoc single credential, or omit -p to use the configured users.
  • sync --rekey removed. The flag (and the rekey= kwarg on sync_html_keys) was a no-op in the v4 format — every sync already regenerates the CEK and produces fresh ciphertext. The dead flag is gone; behavior is unchanged.