dcosnet
/
Vestibule
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
Vestibule/docs/QA-PASS.md

20 KiB
Executable File
Raw Permalink Blame History

Vestibule — QA Production Readiness Pass (1.2.0)

Review date: 2026-08-24 Subject: vestibule-1.2.0 — full tree (extension, usher, scripts, packaging, docs) Reviewer: Mixture-of-Experts panel — ten seats: five disciplines, each with a Linux and a Windows counterpart (QA analyst, platform engineer, architect, administrator, DevOps project manager) Standards applied: PEP 8 (spirit) for Python, POSIX sh, SEI CERT, MISRA-C (spirit), Unix philosophy Method: every finding is either fixed in this pass or recorded under Honest limitations. No finding is deferred silently. Addendum: a 1.2.2 feature pass (safe-by-default navigation) is recorded at the end of this document, same method.

Panel composition

Seat Focus
Senior QA Analyst (Linux / Windows) Test coverage, edge cases, failure modes, protocol correctness, doc-code parity
Senior Linux Engineer D-Bus integration, systemd, packaging, POSIX compliance, shell hygiene
Senior Windows Engineer AssignedAccess/Shell Launcher, WMI bridge, PowerShell 5.1 compatibility, ACLs
Senior Architect Module boundaries, data flow, threading model, table-driven dispatch
Senior Administrator (both platforms) Deployment, configuration, recovery, observability, operability
Project Manager (DevOps) CI/CD, release management, versioning, risk register

Release-blocking findings — all fixed in this pass

  1. Fake constant-time comparison in the admin wizard (extension/admin.js). verifyPassword() used Array.prototype.every(), which short-circuits on the first mismatch, then wrapped the result in a dead ternary — a timing side channel on the admin password check and a SEI CERT MSC06-C violation. Replaced with a single XOR-accumulate reduce() over every byte: no short-circuit, no dead branch, one return.

  2. Version skew across the release. The tree declared 1.1.0 in nine places (Cargo.toml, Cargo.lock, extension manifest, Inno Setup ×2, Flatpak CLI, build-flatpak.sh, metainfo, validate.sh ×3, CI ×5, integration test, docs) while the release artifact is 1.2.0. Every declaration now reads 1.2.0, the metainfo carries a 1.2.0 release entry, and validate.sh enforces the single version across all five machine-readable sources — a mismatch now fails local validation before it can fail in CI.

  3. Documentation contradicted the implementation on credential storage. README, admin wizard UI, TOML schema, and BLOG all claimed the unlock hash lives in the OS keyring; helper/src/storage.rs stores the Argon2id PHC string in a file at 0600 (Windows: default user ACL). The documentation now states the file-storage design and the reason for it: keyring daemons do not exist on minimal kiosk compositors such as cage, so file permissions are the enforcement boundary. Cargo.toml's stale "keyring bridge" description is gone.

  4. Stale Native Messaging manifest path (Windows). config/com.vestibule.usher.windows.json pointed at C:\Program Files\Vestibule\usher.exe; the provisioning scripts stage the binary at ...\Vestibule\bin\usher.exe. Corrected to the staged location. (Live registration always generated its own manifest with the resolved path; the shipped file is the reference copy.)

  5. Shell injection surface in install-native-host.sh. The manifest generation interpolated TARGET_BIN directly into Python source via -c "..." — a path containing a quote would alter the program (SEI CERT IDS03 family). Paths now travel as argv to a heredoc script; the source contains no interpolated values.

  6. Integration-test coverage gap. The prior QA record claimed the launcher dispatch was "asserted, not just parsed" — no such test existed. scripts/test-provision-linux.sh now runs the launcher itself against browser shims for all four env permutations (firefox/flatpak, firefox/native, librewolf/flatpak, defaults) and asserts the exec'd command line. Coverage moved from 122 to 126 assertions, all passing.

Coding standards findings — all fixed in this pass

Standard Finding Fix
MISRA-C (spirit): table-driven dispatch over nested conditionals power.rs wnd_proc used sequential if msg == ... tests Single match — one arm per handled message, default arm defers to DefWindowProcW
SEI CERT: no unwrap() on fallible values power.rs called notify_handle.unwrap() after a non-binding match Handle bound in the match; the failure arm returns with monitoring disabled, exit path never panics
Arrays/tables over nested ifs provision-kiosk.sh carried two near-duplicate resolver functions with nested if/elif ladders One flavor_available lookup table (browser:flavor → test) plus linear step-down resolvers — one line per flavor, first available wins
DRY / Unix philosophy Three separate ordered-path probe loops in provision-kiosk.sh Single first_executable() helper; binary probes are one assignment each
Avoid for/while where a declarative form exists make-icons.py render loop and listing loop; nested small-size branch Dict comprehension for rendering, "\n".join() for the listing, and a step-down draw_icon() that selects draw_simple_icon() or draw_full_icon() — output verified byte-identical on all 8 artifacts
Avoid for/while where a declarative form exists test-native-messaging.py was six copy-pasted sequential blocks Protocol steps are a table (label, request, timeout, criterion); the driver is one comprehension over map(run, STEPS); failure report is a join
POSIX sh Launcher's browser/flavor forks used nested if inside if case dispatch — the fork table is the case statement
PEP 8 Inline hash-file branch, os.path.exists guard before os.remove contextlib.suppress(FileNotFoundError); named timeout constants

Loops that remain, deliberately. Event loops that are the program's purpose are not data-processing loops and have no declarative equivalent: usher's stdin dispatch and channel-draining writer (main.rs), the D-Bus signal drain and reconnect loop (power.rs), the Windows message pump, and the kiosk supervision loop (kiosk-launch.ps1). Argument-parsing while [ $# -gt 0 ] and iteration over data lists in POSIX sh (no arrays in the language) likewise stay — they are the minimal mechanism the platform offers. This is the "where possible" boundary.

Language and tone findings — all fixed in this pass

The pass removed every phrase narrating reversal or back-and-forth history ("restored", "brought back", "honored for back-compat", "pulled forward and re-scoped", "Phase N may move"). Every comment and document now states current behavior in the present tense, as a decision.

Where Before After
vestibule-kiosk-launch VESTIBULE_LIBREWOLF_FLAVOR (pre-Firefox-support name) honored as a fallback Removed. kiosk.env is written by the current provisioner and carries VESTIBULE_BROWSER_FLAVOR; the launcher reads exactly one variable per concern
deprovision-kiosk.sh / .ps1 restore_policies() / "restored original policies.json" reset_policy_dir() / "baseline policies.json reinstalled" — the operation is a baseline reset, stated as one
test-provision-linux.sh "policies.json restored byte-for-byte" "policies.json equals the pre-provision baseline (byte-for-byte)"
DEPLOYMENT.md / README / metainfo "rollback", "full deprovision/rollback" "deprovision" — one noun, one operation
provisioners (both platforms) "falling back to Shell Launcher", "Firefox fallback" "step-down": the choice forks are ordered ladders, documented as such
BLOG.md "Phase 2 shipped — and grew past that plan… pulled forward from Phase 4 and re-scoped" "Vestibule 1.2 ships…" — what the release contains, not how the plan moved
admin wizard UI / admin.js / CSS headers "Phase 0 spike", "In the spike this is not verified", "Phase 1 stores the Argon2id hash in the OS keyring" Production wording matching the shipped behavior

Step-down logic is now the named mechanism at every fork: browser resolution (LibreWolf native → LibreWolf Flatpak → Firefox native → Firefox Flatpak → Firefox snap), lockdown application (AssignedAccess bridge → Shell Launcher), browser discovery on Windows (filesystem paths → HKLM App Paths → HKCU App Paths), and the usher source probe (explicit flag → installed binary → repo build → staged copy). Each is an ordered ladder with first-match-wins; none is a nested conditional.

Verification performed in this pass

All checks were executed in a clean checkout of the pass output; none are quoted from earlier records.

  • scripts/test-provision-linux.sh: 126/126 assertions pass. Three full provision/deprovision scenarios (LibreWolf native, Firefox native, Firefox Flatpak) covering kiosk-user creation, /opt staging, XPI build, launcher install, kiosk.env contents, all five Native Messaging manifest locations with valid JSON and correct paths, policies deep-merge preserving the browser's own keys, force-installed extension with the correct install_url (sandbox-visible for Flatpak), unit generation and enable, and byte-for-byte baseline equality after deprovision — plus the four new launcher-dispatch assertions.
  • scripts/test-native-messaging.py (table-driven rewrite) verified end-to-end against a protocol-faithful mock helper: all six steps pass with exit 0; a deliberately corrupted hello response produces exit 1 with a precise field-level message. (The Rust toolchain is not available in this review environment, so the mock stands in for the binary; CI builds and exercises the real usher on both platforms.)
  • scripts/make-icons.py refactor is rendering-equivalent: all 8 generated artifacts (SVG, six PNGs, ICO) are byte-identical to the committed set (SHA-256 comparison).
  • Syntax gates: sh -n (dash) on every shell script including the launcher and Flatpak CLI; python3 -m py_compile on both Python scripts; JSON validity on all five JSON files; XML validity on the metainfo; YAML validity on the CI workflow; PowerShell structural checks (here-string placement, brace/paren/bracket balance).
  • scripts/validate.sh passes end-to-end on the tree, including the new 1.2.0 version-consistency gate.
  • Doc-code parity spot checks: every console log line quoted in QUICKSTART/DEPLOYMENT now matches the strings the code emits; the verification checklist version matches the manifest.

Honest limitations

  1. Windows lockdown paths need hardware validation. The MDM WMI bridge call, AUMID shortcut property set, and Shell Launcher step-down are implemented to documentation and edition-gated, but this pass exercised them only through parse/structure checks — no physical Windows Pro/Ent machine was available. The Linux path is integration-tested; the Windows path is CI-validated for syntax and structure only.
  2. Rust changes compile-verified by review, not by build. This environment has no cargo; the power.rs match-dispatch and handle-binding changes and the 1.2.0 version bump follow patterns the existing CI matrix compiles on every push, but the binaries in this tarball were not rebuilt here.
  3. Firefox snap and Flatpak policy paths follow Mozilla's documented sandbox layouts and are provisioned and deprovisioned deterministically, but were validated against shims, not real snap/Flatpak Firefox installs as a system-wide kiosk user.
  4. The installer and usher binary remain unsigned — a standing decision, not an oversight. A code-signing certificate is an unfunded line item unless a donation earmarks it; SmartScreen warns and operators verify the SHA-256 from the release notes. This decision is the root of the Linux-first platform priority (see Post-pass decisions).
  5. Windows autologon stores the kiosk password in plaintext registry — accepted trade-off on a locked-down appliance, documented with the -NoAutoLogon escape.
  6. Flatpak LibreWolf policies are wiped by app updates (the distribution dir lives inside the flatpak). Re-run provisioning after updates; documented in DEPLOYMENT.md.

Post-pass decisions (2026-08-24)

  1. Platform priority: Linux. The Windows stack stays complete and CI-validated, but its binaries stay unsigned: a code-signing certificate is an unfunded line item and remains one unless a donation earmarks it (info@dcos.net). Linux carries no signing gate and is the supported production path. README, DEPLOYMENT.md, and BLOG.md now state this decision where each document introduces the platform story.
  2. Historic artifact: the 2001 VB6 original. The original source is included under history/vb6-2001/ — all three iterations (earliest, lobby build, shipped final), scrubbed of employer, school, and client identifiers: employer-branded project filenames renamed to the neutral LobbyBrowser*, the company version string emptied, the hardcoded unlock code redacted, the 2001 readme's employer mention rewritten to the sanitized provenance (co-op while in school, first employer — a Boston-area MSP and programming company). The compiled binary, a captured copy of IEXPLORE.EXE, and an empty scratch file are excluded (a binary cannot be scrubbed without a rebuild; the IE executable is Microsoft's, not authored code). scripts/validate.sh gates the scrub: the identifier scan fails the release if any of it reappears. The lock screen's "System Security 1.5" branding strings are left in place — they are what the shipped binary displayed, and they document the sibling app whose UI the browser reused.

Risk register

Risk Likelihood Impact Mitigation
LibreWolf/Firefox ESR drops webRequestBlocking Low Critical Test in ESR beta CI; declarativeNetRequest is the designated step-down
zbus 5.x breaking change Medium Medium zbus pinned to major 4; test on each minor bump
MDM WMI bridge unavailable on locked-down SKUs Medium Medium Shell Launcher step-down (Ent/Edu); manual Settings path documented
D-Bus unavailable in container/CI High (CI) Low Power thread logs and retries; kiosk functions without power events
Windows code-signing cert cost High Medium Unfunded by decision; a donation earmarked for a certificate flips it. Linux is the primary target in the meantime

Verdict

1.2.0 is production-ready within the documented limitations. The release blockers (credential-comparison side channel, version skew, doc-code contradictions) are closed; the coding-standards sweep is applied across Python, shell, PowerShell, Rust, and JavaScript; every choice fork in the deployment surface is an ordered, named step-down ladder; the test suite grew to 126 assertions and passes clean; and the documentation reads as a decision log in the present tense. The panel approves the release.

Addendum — 1.2.2 safe-by-default navigation (2026-08-24)

Scope: the URL policy gains a safelist mode that is the new default: every domain is blocked — top-level pages, frames, and subresources alike — until the operator lists it. Matching is hostname-based; the legacy substring modes remain unchanged for existing deployments.

Design decisions (all present tense, all enforced in code)

  1. Domain matching, not substring matching. The entry example.org grants the domain and its subdomains and nothing else. Query-string, path-segment, and userinfo smuggles (https://evil.com/?q=example.org, https://example.org@evil.com/) that pass the legacy allowlist are blocked by the safelist. This smuggle class is asserted explicitly in the unit suite.
  2. Internal schemes are always permitted (about:, moz-extension:, chrome:, resource:); data: and blob: are permitted as subresources and blocked as top-level documents. An empty safelist therefore degrades to a kiosk showing about:blank, not a kiosk showing nothing.
  3. Home-origin guarantee. The engine exempts the configured home URL's hostname in every mode, so reset/wake/idle navigation can never strand the kiosk on its own block page. The wizard enforces the same rule at save time for the home and attract URLs and live on step 1, with a one-click add-to-safelist button.
  4. First-boot adoption. A provisioned kiosk launches with its home URL on the command line, which browser storage cannot know. While the policy is still the default, the extension adopts the browser's startup page as home and safelists its domain. Only tabs present at background startup qualify — never a later typed navigation — so the walk-up attack (type your own domain on an unconfigured kiosk) does not apply.
  5. Fail-closed dispatch. An unrecognized mode string resolves to the safelist predicate. 1.2.0 failed open; the panel reversed that for 1.2.2 — a corrupted policy locks the kiosk to its home page rather than opening the perimeter.
  6. Blocked top-level navigations land on an in-extension block page (blocked.html) that names the blocked domain via textContent only — no reflection of the URL parameter into markup. Blocked subresources are cancelled outright.
  7. One engine, three consumers. The decision logic lives in extension/url-policy.js (pure, browser-API-free, UMD-lite export): the background script's webRequest wiring, the wizard's live validation and save-time check, and the Node unit suite all call the same functions. The wizard cannot disagree with runtime enforcement.

Verification performed in this pass

  • scripts/test-url-policy.js: 62/62 assertions pass under Node 24 — entry normalization (domains, wildcards, pasted URLs, ports, junk), home-hostname extraction, safelist matching (exact, subdomain, deep subdomain, sibling non-match, three smuggle classes, empty-hostname file:), internal-scheme and data:/blob: handling, empty-safelist posture, home-origin guarantee (exact match, subdomain non-coverage, suffix-spoof non-match), legacy-mode behavior parity (including the documented empty-allowlist quirk), fail-closed unknown mode, and startup adoption (positive case plus five refusal cases).
  • The unit suite caught one real defect before ship: the data:/blob: branch of the safelist predicate was inverted (!ctx.mainFrame instead of ctx.mainFrame), which would have allowed top-level data: navigations and blocked data: subresources. Fixed; the four assertions now pin the correct polarity.
  • node --check on all seven extension scripts; sh -n on validate.sh after the new gate; JSON/XML validity re-verified by the full scripts/validate.sh run (all gates green, including the new version-consistency gate at 1.2.2 and the new URL-policy gate).
  • XPI smoke test now asserts url-policy.js loads first in the background script array and that blocked.html ships in the bundle.
  • Version consistency: 1.2.2 declared in Cargo.toml, Cargo.lock (usher row only — static_assertions legitimately pins 1.2.0), extension manifest, Inno Setup ×2, Flatpak CLI, build-flatpak.sh, metainfo (new 1.2.2 release entry), validate.sh ×3, CI ×5, build-installer.ps1, integration test, QUICKSTART console lines, DEPLOYMENT artifacts table and checklists.

Honest limitations (1.2.2)

  1. Safelist mode gates subresources like navigations. A safelisted page pulling scripts or fonts from a CDN renders broken until the CDN domain is listed. Deliberate (data can leave through subresource requests) and documented in the wizard help text, README, and QUICKSTART — but the operator's first configuration pass will involve adding domains until the page renders whole. The console logs each block with the exact URL.
  2. Startup adoption inspects the tab list once at background boot. A browser that restores a session (non-kiosk deployment) adopts the first restored http(s) tab while the policy is default. Kiosk deployments boot to a single page; the behavior is correct there, and any wizard save ends adoption permanently.
  3. The redirect to blocked.html uses webRequest redirectUrl, which discards POST state on blocked form submissions. Acceptable — the submission was refused either way — and irrelevant to GET-based kiosk content.
  4. Engine tests run under Node; the webRequest wiring itself is CI-exercised, not unit-exercised. The wiring is 20 lines of state-and-listener code; the decision table it delegates to is fully covered.

Review conducted 2026-08-24 by the Mixture-of-Experts panel. Author: Jeremy Anderson — dcos.net — info@dcos.net.