chore: Fix 49 vulnerabilities, repair two crash bugs, restore CI - #2
Merged
Conversation
The repository had no .gitignore, so a plain `npm install` left node_modules/ showing as untracked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`add()` tested `y > location.highX` when deciding whether a point fell below the child matrix. For square children highX === highY so the typo was invisible, and every existing test used a square child. For a non-square child the branch takes the wrong path: the row is never created and `points[z][y].push(x)` throws `TypeError: Cannot read properties of undefined (reading 'push')`. Row creation was also coupled to the bound-extending branch, so a second point on an already extended row hit the same crash even when the bounds were correct. Rows are now created on demand, independently of the bounds update. Adds two regression tests, both of which fail on the previous code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three defects in README.md: * The step 1 and step 2 snippets both referenced `filter.length`, but no `filter` variable exists in either — copy-pasting threw a ReferenceError. example.js uses `x < 2`, which is what was meant. * Step 2 passed the weights coordinates to `.at()` and the filters coordinates to `.add()`, which is backwards: the parent log is the 2x2 filters matrix, so `.at()` was indexed out of bounds and threw. Swapping them makes the snippet produce exactly the output the README documents, verified byte for byte, and matches example.js. This part of the change is taken from the fix proposed in #1 by Massimiliano Filacchioni (@mfila). * The API synopsis imported `'matrix-log'` (the package is published as `matrix-log.js`) via `import * as MatrixLog`, which yields a module namespace object rather than the exported class, so `new MatrixLog()` could not construct. Also adds the missing installation section. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jest 23 (Aug 2018) pulled in a dev tree with 49 known advisories (15 critical, 13 high, 21 moderate) via babel-core 6, request, jsdom, node-notifier, tough-cookie and friends. jest 30 drops all of them. The test suite is runner-agnostic (describe/it/expect only) and needed no changes: 38/38 pass before and after. Also: * Drop `--env=node` from the test script; node has been jest's default test environment since jest 27. * Declare `engines.node` >= 18.14, the floor jest 30 supports. * Commit package-lock.json. Without one, Dependabot only sees the single declared devDependency and reports nothing for the transitive tree, which is why the alert list was empty despite 49 live advisories. Remaining afterwards: 19 high, which are all one advisory, GHSA-mh99-v99m-4gvg (brace-expansion DoS via unbounded expansion), counted once per dependent. It is deliberately NOT pinned: * The only patched release is brace-expansion 5.0.8; nothing was backported to the 1.x or 2.x maintenance lines that jest resolves to. * 5.0.8 changes the CommonJS export from a callable default to `{ expand, EXPANSION_MAX, EXPANSION_MAX_LENGTH }`. Both consumers here (minimatch 3 via test-exclude, minimatch 9 via glob) call it as a function, so an `overrides` pin yields `TypeError: expand is not a function` on any brace pattern. This was tested: the suite still passes under the pin only because no glob in this repo uses braces, so the pin buys nothing and arms a landmine for the first `*.{js,ts}` pattern. * It is devDependency-only and unreachable by consumers: this package publishes `files: [index.js]` and has no runtime dependencies. * Impact is availability-only (C:N/I:N/A:H) and needs an attacker controlled glob; globs here come solely from jest's own config. Revisit once jest's minimatch/glob chain moves to brace-expansion 5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
example.js and the matching README step used the gpu.js 1.x default
export (`const GPU = require('gpu.js')` / `import GPU from 'gpu.js'`).
gpu.js 2.0 moved to named exports, so step 5 died on
`TypeError: GPU is not a constructor` — the example has been broken
against every gpu.js release since.
With the constructor resolved, step 5 computes the right numbers but
returns Float32Array rows, which `assert.deepStrictEqual` rejects on type
against the plain-array expectation. Normalise the rows before comparing.
Verified end to end against gpu.js 2.19.8: `node example.js` now exits 0
with all five steps agreeing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
.travis.yml targeted Node 6, 8 and 10 — all EOL for years — and travis-ci.org shut down for open source in 2021, so this repository has had no working CI since. There were no Actions workflows to replace it. Adds .github/workflows/test.yml running `npm ci && npm test` across Node 18/20/22/24 on push, pull_request and manual dispatch, with a read-only GITHUB_TOKEN and npm caching. `npm ci` is now usable because a lockfile is committed. example.js is intentionally excluded: its last step needs gpu.js, whose `gl` dependency is a native module needing system GL headers on the runner, which would fail for reasons unrelated to this package. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
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
1 participant
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
Security remediation and modernization for a package whose last commit was October 2018 and whose CI (Travis) has been dead since 2021. Also fixes two real bugs found along the way. The public API is unchanged — the
index.jsdiff is 4 lines net insideadd().Security
GET /dependabot/alertsreturned[], but that was a false negative rather than a clean bill of health: with no lockfile committed, Dependabot only ever saw the single declared devDependency and never scanned the transitive tree. Auditing the resolved tree locally found the real picture.npm auditnpm testAll 49 were cleared by upgrading jest 23 → 30. Committing
package-lock.jsoncloses the visibility gap so Dependabot can actually see transitives from now on.The 19 remaining are one advisory, deliberately not pinned
GHSA-mh99-v99m-4gvg (
brace-expansionDoS,<=5.0.7), counted once per dependent. Anoverridespin was tried and reverted:{ expand, … }. With the pin the suite still passed, but only because no glob here uses braces —minimatch@3then throwsTypeError: expand is not a functionon any*.{js,ts}pattern. The pin buys nothing and arms a landmine.files: ["index.js"], zero runtime dependencies). Availability-only impact requiring an attacker-controlled glob.Bugs fixed
add()comparedyagainsthighXwhen growing the vertical bound. Invisible for square children — every existing test used 4×4 — but for a non-square child the row is never created andpoints[z][y].push(x)throwsTypeError: Cannot read properties of undefined (reading 'push'). Row creation was also coupled to the bound-extending branch, so a second point on an already-extended row crashed too. Both fixed, with 2 regression tests confirmed to fail against the unpatched code with that exact error.example.jshas been broken since gpu.js 2.0. It used the 1.x default export (const GPU = require('gpu.js')), which throwsGPU is not a constructor. Nowconst { GPU } = …, with the kernel'sFloat32Arrayrows normalized beforedeepStrictEqual. Verified:node example.jsexits 0 against gpu.js 2.19.8 with all five steps agreeing.README step 2 did not run. Incorporates #1 by @mfila — master passed weights coords to
.at()and filter coords to.add(), which is backwards, so.at()indexed the 2×2 parent out of bounds and threw. That change is correct and reproduces the README's own documented output byte-for-byte. It left a second defect untouched, though: both snippets referencefilter.length, and nofiltervariable exists, so the code still threwReferenceErroron paste. Both are fixed here. Also corrected the wrong package name and a non-constructableimport * as MatrixLog from 'matrix-log', and added the missing install section.Modernization
npm ci && npm teston Node 18/20/22/24, read-only token, npm cache. CI badge added.--env=node(node is the default since jest 27).engines.node >= 18.14,.gitignore, and the lockfile.No
example.jsCI job: gpu.js'sglnative dependency needs system GL headers on the runner and would fail for reasons unrelated to this package. Noted in a comment in the workflow.Testing
npm test→ 38/38 pass (36 original + 2 new). No pre-existing failures — jest 23 ran clean on Node 22 before any changes, so all 36 original tests passed both before and after; no failures were introduced.npm cifrom the committed lockfile verified clean. The 2 new tests were confirmed to fail against the unpatchedindex.jswith the exact expectedTypeError.Deliberately out of scope: no rewrite, no TypeScript/ESM migration, no version bump.
🤖 Generated with Claude Code