fix(security): pin vendored model-viewer bundle to verified upstream - #2709
Open
superm1 wants to merge 1 commit into
Open
fix(security): pin vendored model-viewer bundle to verified upstream#2709superm1 wants to merge 1 commit into
superm1 wants to merge 1 commit into
Conversation
6 tasks
fl0rianr
requested changes
Jul 15, 2026
fl0rianr
left a comment
Collaborator
There was a problem hiding this comment.
Thanks, right direction, but not ready yet:
The vendored model-viewer.min.js had drifted from the documented upstream (@google/model-viewer@4.3.1) and carried no integrity hash, so a webpack import could not detect tampering or corruption. Replace it with the pristine bundle extracted from the npm registry tarball, pin its SHA-256 in a sidecar checked by CI, and document the hash chain plus a hash-preserving update process that verifies npm's dist.integrity.
superm1
force-pushed
the
fix/model-viewer-integrity
branch
from
July 28, 2026 04:28
e1bb81e to
0e6e28b
Compare
fl0rianr
reviewed
Jul 28, 2026
fl0rianr
left a comment
Collaborator
There was a problem hiding this comment.
Thanks, better, but not fully there yet.
| # (2) Independently verify npm's published integrity hash. | ||
| EXPECTED=$(curl -s "https://registry.npmjs.org/@google/model-viewer/${VERSION}" \ | ||
| | python3 -c "import sys,json; print(json.load(sys.stdin)['dist']['integrity'])") | ||
| ACTUAL=$(sha512sum "$TARBALL" | awk '{print "sha512:" $1}') |
Collaborator
There was a problem hiding this comment.
blocking: EXPECTED and ACTUAL use different digest representations. npm's dist.integrity is formatted as sha512-<base64>, while sha512sum produces a hexadecimal digest and this code prefixes it with sha512. As a result, the documented update process rejects even the correct upstream tarball.
Generate ACTUAL in npm's SRI representation instead:
Suggested change
| ACTUAL=$(sha512sum "$TARBALL" | awk '{print "sha512:" $1}') | |
| ACTUAL=$(python3 - "$TARBALL" <<'PY' | |
| import base64 | |
| import hashlib | |
| import pathlib | |
| import sys | |
| digest = hashlib.sha512(pathlib.Path(sys.argv[1]).read_bytes()).digest() | |
| print("sha512-" + base64.b64encode(digest).decode("ascii")) | |
| PY | |
| ) |
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
The vendored
src/app/src/renderer/vendor/model-viewer.min.jshad drifted from the documented upstream and carried no integrity hash, so a webpack import could not detect tampering or corruption.This PR pins to the pristine artifact and secures it:
dist/model-viewer.min.jsfrom the npm registry tarball for@google/model-viewer@4.3.1, verified against npm's published sha512 integrity.model-viewer.min.js.sha256(SHA-256283b06…) and a CI job indocs_and_style.ymlthat runssha256sum -cto fail on any drift.README.mdto record the hash chain (npm tarball integrity → extracted-file SHA-256) and a hash-preserving update process that validates npm's dist.integrity before extraction.Review notes
Shadow mapping fix (upstream #5168)
The locally patched 4.3.0 file contained a shadow-depth shader fix from google/model-viewer#5168 correcting the orthographic frustum and floor/blur-plane scaling for models larger than 1×1. That fix is included in the official 4.3.1 release, so the bundle is now pristine upstream with the fix intact.
"Drift guard" vs SRI
The sidecar SHA-256 check is a drift guard rather than a true SRI replacement: the file and checksum can be changed together in the same commit. The npm dist.integrity (SHA-512) is the authoritative upstream trust anchor, and the documented update process verifies it before extracting the file.