Conversation
Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…i-validation fix(GltfLoader): pass percent-decoded URI to resolveUri callback to prevent SSRF bypass
Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
fix(ShaderCache): hash GLSL source to derive shader cache keys instead of storing raw strings
Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
…ed-input fix: enforce maxJsonBufferBytes on decoded string before JSON.parse
…ne where no scripts are loaded Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
…-drift test for demo.html Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
CSP: replace bare `script-src 'self'` with hash+strict-dynamic or `'none'`
… keys > MAX_KEY_LENGTH (512) Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
…key-length fix(ShaderCache): bound explicit cache key length to prevent unbounded Map allocation
… catastrophic backtracking Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
…error Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
…ernal-uri fix(security): add URI length cap in validateExternalUri to prevent ReDoS
Contributor
There was a problem hiding this comment.
Pull request overview
Release v1.1.14 focuses on tightening security controls at the HTML entrypoints (CSP hardening) and strengthening input/resource validation in core subsystems (glTF loading + shader/program caching).
Changes:
- Hardened CSP across public HTML pages (no bare
script-src 'self'; hash +strict-dynamicbootstrap fordemo.html). - Strengthened core validation: bounded JSON parsing guardrails in
GltfLoaderand bounded explicit cache key sizes + hash-based shader keys inShaderCache. - Added/updated tests and documentation to enforce CSP invariants and validate new loader/cache behaviors.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
index.html |
CSP updated to script-src 'none' for a no-script page. |
demos.html |
CSP updated to script-src 'none' for a no-script page. |
gallery.html |
CSP updated to script-src 'none' for a no-script page. |
demo.html |
CSP switched to hash + strict-dynamic; bootstraps via inline module import. |
SECURITY.md |
Documents CSP approach, rationale, and reporting guidance. |
tests/csp.test.ts |
Adds checks preventing bare script-src 'self' and validates demo.html hash freshness. |
src/core/GltfLoader.ts |
Adds decoded-string size guard; adds strict URI max length; adds bounded full percent-decoding + dual-form URI validation before resolveUri. |
tests/gltf.test.ts |
Adds regression tests for decoded-string guard and strict URI length/decoding behavior; updates payload limit expectation. |
src/core/ShaderCache.ts |
Introduces MAX_KEY_LENGTH for explicit keys; defaults shader keys to hash-derived strings instead of raw source. |
tests/shader.test.ts |
Adds tests for hashed default shader keys and explicit key length validation. |
package.json |
Bumps version to 1.1.14. |
package-lock.json |
Bumps lockfile package version to 1.1.14. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
Author
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Contributor
|
@ormidales I've opened a new pull request, #359, to work on those changes. Once the pull request is ready, I'll request review from you. |
8 tasks
- Split maxJsonBufferBytes into separate raw-byte and decoded-string limits (new maxJsonStringBytes option, defaults to maxJsonBufferBytes * 2) to avoid unexpectedly halving the ASCII payload limit - Apply maxJsonBufferBytes and maxJsonStringBytes bounds inside parseGlb for the GLB JSON chunk, closing the resource-exhaustion gap on the GLB path - Centralize explicit key length validation in ShaderCache.assertKeyLength() private helper; replace three duplicated inline checks in getShader, getProgram, and getProgramKey - Update tests: rename string-guard test to use maxJsonStringBytes, add independence test, and add two GLB JSON chunk size limit tests Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
…tle test - Validate maxJsonBufferBytes and maxJsonStringBytes in parseContainer: NaN, Infinity, or negative values now throw a RangeError immediately, preventing the size guards from being silently disabled - Add chunk length overflow bounds check in parseGlb: verify offset+8+chunkLength <= buffer.byteLength before slicing; malformed or truncated GLBs now throw a clear 'extends beyond end of file' error - Fix brittle GLB chunk size test: use getUint32(12, true) to read the actual on-disk JSON chunk length from the GLB header instead of re-stringifying the parsed result (which can differ due to padding) - Add 6 new tests: NaN/Infinity/negative for both limits, and chunk-extends-beyond-file Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
Fix GltfLoader JSON size limits and ShaderCache key validation centralization
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.
This pull request introduces major improvements to security policies and resource validation throughout the project, focusing on stricter Content Security Policy (CSP) enforcement for all HTML pages and enhanced URI and cache key validation in core loader and shader modules. These changes help prevent common web vulnerabilities, tighten resource access, and ensure robust handling of external inputs.
Security policy enhancements:
index.html,demos.html,gallery.html,demo.html) now use a strict CSP meta tag, removing bare'self'fromscript-srcand replacing it with'none'for pages without scripts, and a hash-based policy plus'strict-dynamic'fordemo.htmlto only allow the known bootstrap import. This prevents unauthorized script execution and mitigates XSS risks. [1] [2] [3] [4] [5]SECURITY.mdfile documents the CSP approach, rationale, and procedures for reporting vulnerabilities, providing clear guidance for maintainers and contributors.csp.test.tsverify that no page uses a bare'self'inscript-srcwithout a nonce or hash, and that the hash indemo.htmlmatches the actual inline script content. [1] [2]Resource validation improvements:
src/core/GltfLoader.ts) now enforces a maximum URI length and performs bounded, full percent-decoding of external buffer URIs, validating both the original and decoded forms to prevent path traversal and SSRF vulnerabilities. The loader also checks the decoded JSON string's heap footprint before parsing, guarding against resource exhaustion. [1] [2] [3] [4] [5] [6]src/core/ShaderCache.ts) introduces a maximum key length for explicit cache keys and uses compact FNV-1a hashes for auto-generated keys, ensuring predictable, bounded cache key sizes and preventing accidental resource exhaustion or collisions. [1] [2] [3] [4] [5] [6]