Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
Two unresolved critical launcher-boundary issues, a moderate hostname-validation issue, and a Node-version documentation mismatch remain.
Pull request overview
Adds an optional AppLoad authentication browser for native phone/QR passkey assertions, a Rust caBLE helper, and a disposable relying-party demo. The standard browser path remains unchanged.
Changes:
- Added native WPE/WebAuthn integration, ephemeral runtime handling, and AppLoad UI.
- Added demo verification, packaging, pinned build inputs, documentation, and tests.
- Added CI and native/WPE/AppLoad regression coverage.
File summaries
| File | Review summary |
|---|---|
tools/passkey-acceptance/test/server.test.js |
Server verification tests |
tools/passkey-acceptance/test/harness.test.js |
Harness validation tests |
tools/passkey-acceptance/test/demo.test.js |
Demo flow tests |
tools/passkey-acceptance/test/credentials.js |
Credential fixtures |
tools/passkey-acceptance/server.js |
Disposable relying-party verifier |
tools/passkey-acceptance/public/style.css |
Demo styling |
tools/passkey-acceptance/public/index.html |
Demo page |
tools/passkey-acceptance/package.json |
Node package metadata; related documentation must match >=22 (nit, 2 votes) |
tools/passkey-acceptance/native/test-preparation.py |
Native preparation checks |
tools/passkey-acceptance/native/test-entry.py |
Native entry tests |
tools/passkey-acceptance/native/test-engine.sh |
Native engine test script |
tools/passkey-acceptance/native/run-engine.py |
Native engine runner |
tools/passkey-acceptance/native/policy.h |
Native policy declarations |
tools/passkey-acceptance/native/policy-test.cpp |
Native policy tests |
tools/passkey-acceptance/native/launcher.cpp |
Native launcher |
tools/passkey-acceptance/native/launch |
Acceptance package launcher |
tools/passkey-acceptance/native/engine-test.cpp |
Native engine fixtures |
tools/passkey-acceptance/native/CMakeLists.txt |
Native build configuration |
tools/passkey-acceptance/native/catalog.py |
Package catalog; reject DNS names over 253 octets at construction (moderate, 2 votes) |
tools/passkey-acceptance/native/build.sh |
Native build script |
tools/passkey-acceptance/harness.js |
WebAuthn verification harness |
tools/passkey-acceptance/DEMO.md |
Demo procedure |
tools/passkey-acceptance/demo.js |
Demo/tunnel runner |
tools/passkey-acceptance/.gitignore |
Demo ignore rules |
toolchain/Dockerfile.app-only-sdk-3.28 |
Pinned SDK image |
tests/qtfb/CMakeLists.txt |
QTFB test target |
tests/auth-wpe-smoke/run.sh |
WPE smoke runner |
tests/auth-wpe-smoke/README.md |
WPE smoke documentation |
tests/auth-wpe-smoke/CMakeLists.txt |
WPE smoke build configuration |
tests/auth-webauthn-provider/run-diagnostic-cases.sh |
Provider diagnostic runner |
tests/auth-webauthn-provider/run-cases.sh |
Provider case runner |
tests/auth-webauthn-provider/README.md |
Provider documentation |
tests/auth-webauthn-provider/https-server.py |
HTTPS fixture server |
tests/auth-webauthn-provider/CMakeLists.txt |
Provider build configuration |
tests/auth-surface/CMakeLists.txt |
Auth surface build configuration |
tests/auth-policy/CMakeLists.txt |
Auth policy build configuration |
tests/auth-passkey/CMakeLists.txt |
Auth passkey build configuration |
tests/auth-passkey-browser/run-cases.sh |
Browser case runner |
tests/auth-passkey-browser/README.md |
Browser test documentation |
tests/auth-passkey-browser/CMakeLists.txt |
Browser test build configuration |
tests/auth_wpe_smoke.cpp |
WPE smoke test source |
tests/auth_launcher_test.py |
Launcher tests |
tests/auth_entry_test.py |
Entry tests |
tests/auth_build_recipe_test.py |
Build recipe tests |
scripts/run-tests.sh |
Test orchestration |
scripts/build-auth-engine-container.sh |
Isolated engine container build |
scripts/app_only_cache.py |
Cache ownership validation |
README.md |
Project overview |
patches/wpe-2.48.5-native-assertion-provider-files.json |
Patched-source hashes |
patches/README.md |
Patch documentation |
patches/appload.md |
AppLoad prerequisites |
patches/appload-v0.5.3-touch-lifecycle.patch |
AppLoad touch-lifecycle patch |
patches/appload-v0.5.3-no-key-logging.patch |
AppLoad key-logging patch |
patches/appload-tests/CMakeLists.txt |
AppLoad regression fixture configuration |
NOTICE |
Third-party notices |
engine/wpeqt/qtfbclient.h |
QTFB transport interface |
engine/wpeqt/CMakeLists.txt |
Optional auth target configuration |
engine/wpeqt/auth-surface.h |
Auth surface interface |
engine/wpeqt/auth-policy.h |
Auth policy interface |
engine/wpeqt/auth-passkey.h |
Auth passkey interface |
engine/auth-passkey-helper/vendor/LICENSE-rmweb |
Helper license |
engine/auth-passkey-helper/tests/process.py |
Helper process tests |
engine/auth-passkey-helper/tests/package_test.py |
Helper packaging tests |
engine/auth-passkey-helper/src/main.rs |
Helper lifecycle |
engine/auth-passkey-helper/src/lib.rs |
Rust module exports |
engine/auth-passkey-helper/src/ceremony.rs |
caBLE ceremony |
engine/auth-passkey-helper/sdk-linker.sh |
SDK linker wrapper |
engine/auth-passkey-helper/README.md |
Helper documentation |
engine/auth-passkey-helper/prepare.py |
Pinned source preparation |
engine/auth-passkey-helper/patches/libwebauthn-buffered-response.patch |
WebAuthn dependency patch |
engine/auth-passkey-helper/package.py |
Helper packaging |
engine/auth-passkey-helper/Cargo.toml |
Rust dependency metadata |
engine/auth-passkey-helper/build-sdk.sh |
Helper SDK build |
engine/auth-passkey-helper/.gitignore |
Helper ignore rules |
device/auth/entry |
Critical (2 votes): validate ownership and no-write permissions before sourcing the environment or executing the entry |
.gitignore |
Repository ignore rules |
.github/workflows/tests.yml |
CI workflow |
.gitattributes |
Git attributes |
Review details
Files not reviewed (1)
- tools/passkey-acceptance/package-lock.json: Generated file
- Files reviewed: 107/113 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+19
to
+26
| [ -f "$auth_root/bin/rmweb-auth-entry" ] && [ ! -L "$auth_root/bin/rmweb-auth-entry" ] \ | ||
| && [ -x "$auth_root/bin/rmweb-auth-entry" ] || fail | ||
| [ -f "$auth_runtime/rmweb-env.sh" ] && [ ! -L "$auth_runtime/rmweb-env.sh" ] || fail | ||
| unset LD_PRELOAD RMWEB_JSC_OPTS RMWEB_JIT RMWEB_SKIA_THREADS | ||
| RMWEB_AUTH_RUNTIME=$auth_runtime | ||
| export RMWEB_AUTH_RUNTIME | ||
| # shellcheck source=/dev/null | ||
| . "$auth_runtime/rmweb-env.sh" |
Comment on lines
+13
to
+20
| for executable in rmweb-auth-entry rmweb-auth-browser rmweb-auth-passkey; do | ||
| [ -f "$A/bin/$executable" ] && [ ! -L "$A/bin/$executable" ] && [ -x "$A/bin/$executable" ] || fail | ||
| done | ||
| [ -f "$R/rmweb-env.sh" ] && [ ! -L "$R/rmweb-env.sh" ] || fail | ||
| unset LD_PRELOAD RMWEB_JSC_OPTS RMWEB_JIT RMWEB_SKIA_THREADS | ||
| export RMWEB_AUTH_RUNTIME="$R" | ||
| # shellcheck source=/dev/null | ||
| . "$R/rmweb-env.sh" |
| url = urlsplit(origin) | ||
| if (url.scheme != "https" or url.path or url.query or url.fragment | ||
| or url.username is not None or url.password is not None | ||
| or not url.hostname or "." not in url.hostname |
Comment on lines
+21
to
+22
| The package metadata requires Node >=20; we use the official documentation's | ||
| Node LTS22+ recommendation. No browser polyfill or virtual authenticator is used. |
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.
Summary
Implementation
The WebKit provider retains secure-context, frame/origin, RP, activation, cancellation, and request-binding checks. It passes WebKit's exact client-data hash to a bounded, one-request helper over private pipes. The native UI presents the hybrid QR and request status; no page-level WebAuthn shim is used.
The browser, entry executable, helper, and runtime are relocatable within an owned installation. The helper restricts Bluetooth handling to the qualified Paper Pro hardware and attempts to restore adapter state after each request. AppLoad lifetime/input fixes and key-label logging removal are supplied separately with pinned prerequisites and regression tests; firmware-specific hooks and boot integration are not included.
Recommended review order:
docs/auth-browser.mdandpatches/appload.md; WebKit provider patch andengine/auth-passkey-helper; native browser/input code; build/package scripts; demo and tests. Much of the line count is the pinned Public Suffix List, Cargo lockfile, third-party notices, and tests.Testing
Automated/build checks run on the extracted upstream branch:
scripts/run-tests.sh: passed, including 54 Node tests and the existing browser host suite.Physical evidence: before extracting this upstream branch, the self-contained demo package and private namespace were checked on a Paper Pro running 3.28.0.172 / Qt 6.10.3. The tester confirmed the tablet displayed PASS after phone QR approval. The extracted branch was rebuilt and regression-tested separately; it has not received a second physical run. Existing verified patched engine libraries were reused, rather than rebuilding the full engine during extraction.
Risks