Security: SCA upgrades and tooling modernization#3
Conversation
…, `moment@^2.30.1`
Lots of other moving pieces here due to the tighter typing inside `immutable`...
Apparently we just never ran the tests on the last version? 😅
…-mmjv-4gpg` by pulling the latest `lodash@^4.18.1`
…he latest `postcss@^8.5.15`
…he latest `rollup@^2.80.0`
…he latest `karma@^6.4.4`
…ve --environment production` findings.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
The `packageManager: yarn@4.17.0` field in `package.json` requires Corepack to be enabled on the runner. Without it, the globally installed Yarn Classic 1.22.22 refuses to install, erroring with: error This project's package.json defines "packageManager": "yarn@4.17.0". However the current global version of Yarn is 1.22.22. Also switch `yarn install --frozen-lockfile --ignore-engines` (Yarn Classic flags) to `yarn install --immutable` (the Yarn Berry equivalent).
The `StreamUser` class from `getstream` has 6 generic type parameters (`<UserType, ActivityType, CollectionType, ReactionType, ChildReactionType, PersonalizationType>`), but `StreamContextValue.user` and `FeedManagerProps.user` only declared `StreamUser<UT>`, leaving the other 5 type params defaulting to `UR`. The `immutable@^4.3.8` upgrade tightened type inference, exposing this mismatch as TS2322 errors in CI. Fix: pass all 6 type params (`StreamUser<UT, AT, CT, RT, CRT, PT>`) in both type declarations to match the actual usage in `StreamApp`.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
- Add --no-sandbox flag to ChromiumHeadless launcher in karma.config.js (Ubuntu 24.04 AppArmor blocks unprivileged user namespaces). - Pin get-intrinsic to 1.3.0 to avoid async-function@1.0.0's module-sync export condition that breaks Gatsby v2's CJS require() on Node 20+. - Set NODE_OPTIONS=--openssl-legacy-provider for docs-build (webpack v4 uses md4 hash, removed from Node 17+ OpenSSL). - Add gatsby-node.js to force webpack mainFields resolution (CJS before ESM) and provide node module fallbacks for browser bundle.
There was a problem hiding this comment.
3 issues found across 331 files
Not reviewed (too large): docs/render-page.js (~140,350 lines), docs/components/activity/index.html (~212 lines), docs/components/notification-feed/index.html (~198 lines), docs/components/flat-feed/index.html (~104 lines), docs/components/comment-list/index.html (~93 lines), docs/components/like-button/index.html (~92 lines), docs/components/reaction-list/index.html (~83 lines), docs/components/notification-dropdown/index.html (~75 lines), docs/components/dropdown-panel/index.html (~71 lines), docs/components/panel/index.html (~68 lines), and 36 more - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/component---src-components-comment-field-mdx-d18ee7639e20aca509e0.js">
<violation number="1" location="docs/component---src-components-comment-field-mdx-d18ee7639e20aca509e0.js:2">
P3: `await console.log(data)` and `await console.log(t)` await a non-thenable value. `console.log()` returns `undefined`, not a Promise, so the `await` is a no-op. This appears in both the playground code preview string and the runtime component literal — users viewing/copying this example will see a misleading pattern that implies console.log is async.</violation>
</file>
<file name="test-browser-bundle/karma.config.js">
<violation number="1" location="test-browser-bundle/karma.config.js:4">
P2: The `--no-sandbox` flag disables Chromium's process sandbox, a security boundary that isolates renderer processes from the OS. This is common in CI/Docker environments where sandboxing may not work, but it should be a conscious trade-off: if any tests load untrusted content or if this config is reused outside of ephemeral CI, removal of sandbox protection introduces real exposure. Consider documenting why the sandbox is being disabled or adding a comment linking to Chrome sandbox requirements.</violation>
</file>
<file name="docs/component---example-readme-md-bc1cec9ab430e0ecc20c.js">
<violation number="1" location="docs/component---example-readme-md-bc1cec9ab430e0ecc20c.js:1">
P3: Accumulating stale build artifacts: this PR adds a 4th hash-variant of `component---example-readme-md-*.js` while the 3 prior variants remain in the repo. Only the version referenced by the HTML entry point is served; the others are dead code that bloat the repository and create confusion about which is current. Consider cleaning up stale docs build artifacts or adding generated `.js`/`.js.map` files under `docs/` to `.gitignore` and generating them at deploy time.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 200 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
The browser bundle test (karma) requires dist/browser.full-bundle.js, which is gitignored and must be built with yarn build. Previously the test was always failing in CI because the dist/ directory was never created — the Chromium sandbox crash masked this underlying issue.
There was a problem hiding this comment.
All reported issues were addressed across 331 files
Not reviewed (too large): docs/render-page.js (~140,350 lines), docs/components/activity/index.html (~212 lines), docs/components/notification-feed/index.html (~198 lines), docs/components/flat-feed/index.html (~104 lines), docs/components/comment-list/index.html (~93 lines), docs/components/like-button/index.html (~92 lines), docs/components/reaction-list/index.html (~83 lines), docs/components/notification-dropdown/index.html (~75 lines), docs/components/dropdown-panel/index.html (~71 lines), docs/components/panel/index.html (~68 lines), and 36 more - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Note: This PR contains a large number of files. cubic only reviews up to 200 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
Fix all with cubic | Re-trigger cubic
The docz theme imports dist/index.css which is gitignored and must be built with yarn build before the docs build can succeed.
- src/utils/immutable.ts: Narrow getInToJS fallback type from unknown to AnyCollection so the helper's contract matches the .toJS() call. - src/utils/immutable.ts: Use === undefined instead of ?? in typedUpdater to preserve explicit null values (old default-parameter behavior). - src/context/FeedManager.tsx: Guard findIndex against -1 before calling List.remove() in all 4 reaction-removal sites to prevent corrupting local state when the deleted id isn't found. - .yarnrc.yml: Remove npmMinimalAgeGate: 0 to enable Yarn's built-in supply-chain age gate for freshly-published packages. - test-browser-bundle/karma.config.js: Add comment explaining why --no-sandbox is needed (Ubuntu 24.04 AppArmor blocks user namespaces). - gatsby-node.js: Replace global mainFields override with targeted resolve.alias for linkifyjs and linkify-plugin-mention CJS entries, avoiding collateral damage to packages that rely on the browser field.
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/utils/immutable.ts">
<violation number="1" location="src/utils/immutable.ts:28">
P2: `typedUpdater` was changed to only substitute `notSetValue` when `value` is `undefined`, but the JSDoc still says it applies when the existing value is `null`/`undefined`. Additionally, call sites in `FeedManager.tsx` pass collection updaters (e.g., list `unshift` or `remove`) that will throw at runtime if they receive `null`. If the intent is to preserve the previous `null`/`undefined` fallback contract, the check should handle both; otherwise the documentation should be updated to match.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| * semantics: it is applied when the existing value is `null`/`undefined`. | ||
| */ | ||
| export function typedUpdater<T>(updater: (value: T) => T, notSetValue: T): (value: unknown) => unknown { | ||
| return (value) => updater((value === undefined ? notSetValue : value) as T); |
There was a problem hiding this comment.
P2: typedUpdater was changed to only substitute notSetValue when value is undefined, but the JSDoc still says it applies when the existing value is null/undefined. Additionally, call sites in FeedManager.tsx pass collection updaters (e.g., list unshift or remove) that will throw at runtime if they receive null. If the intent is to preserve the previous null/undefined fallback contract, the check should handle both; otherwise the documentation should be updated to match.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/utils/immutable.ts, line 28:
<comment>`typedUpdater` was changed to only substitute `notSetValue` when `value` is `undefined`, but the JSDoc still says it applies when the existing value is `null`/`undefined`. Additionally, call sites in `FeedManager.tsx` pass collection updaters (e.g., list `unshift` or `remove`) that will throw at runtime if they receive `null`. If the intent is to preserve the previous `null`/`undefined` fallback contract, the check should handle both; otherwise the documentation should be updated to match.</comment>
<file context>
@@ -25,10 +25,10 @@ export function getInToJS<T>(source: AnyCollection, keyPath: Iterable<unknown>,
*/
export function typedUpdater<T>(updater: (value: T) => T, notSetValue: T): (value: unknown) => unknown {
- return (value) => updater((value ?? notSetValue) as T);
+ return (value) => updater((value === undefined ? notSetValue : value) as T);
}
</file context>
| return (value) => updater((value === undefined ? notSetValue : value) as T); | |
| return (value) => updater((value == null ? notSetValue : value) as T); |
- Run prettier on FeedManager.tsx and immutable.ts (trailing commas, as-cast parens) - Remove .github/workflows/size-limit.yml — andresz1/size-limit-action@v1 is not on the org's GitHub Actions allowlist, causing a startup_failure on every PR
randytarampi
added
bug
2 participants
Summary
Security-hardening and tooling-modernization work from the
securitizebranch. Clears the outstandingyarn npm auditfindings and brings the build, CI, and toolchain up to date without changing the public component API.Security
moment^2.30.1immutable^4.3.8(adds typedgetIn/updateIn/fromJShelpers insrc/utils/immutable.ts)lodash^4.18.1postcss^8.5.15rollup^2.80.0karma^6.4.4jws^3.2.3+ pinjsonwebtoken@^9.0.2semver,qs,tough-cookie,follow-redirectsgetstream/axios^1resolutionAll remaining
yarn npm audit --no-deprecations --all --recursive --environment productionfindings cleared.Fix
linkify's hashtag linking is restored.init()is now called after both thementionandhashtagplugins are registered, sincelinkifyjs4.x ignores any plugin registered after the firstinit()call.Chore
yarn set version berry,packageManager: yarn@4.17.0)typescript@^5^29(jest,babel-jest,ts-jest,jest-environment-jsdom) and add@types/jest@types/emoji-mart,@types/url-parse,@types/webscopeio__react-textarea-autocompletetodevDependenciesengines.node: >=20)actions/checkout@v4andactions/cache@v4in CI workflowsChanged files (17 files, +3423 / −2407)
Key files:
package.json,yarn.lock,CHANGELOG.md,src/utils/immutable.ts,src/utils/textRenderer.tsx,src/context/FeedManager.tsx,src/types/linkifyjs.d.ts,.github/workflows/*,.yarnrc.ymlNote
The
modernizebranch (PR #4) builds on top of this branch. If this PR merges first, that PR will automatically reduce to just the React 18 modernization commit.Summary by cubic
Security hardening and tooling modernization that clear all
yarn npm auditfindings, restore hashtag linking, and stabilize CI/docs builds on Node 20–26, removing thesize-limitworkflow that blocked CI. No public component API changes.Dependencies
moment@^2.30.1,immutable@^4.3.8,lodash@^4.18.1,postcss@^8,rollup@^2,karma@^6; pinnedjsonwebtoken@^9.0.2withjws@^3.2.3; consolidatedsemver/qs/tough-cookie/follow-redirects; added agetstream/axios@^1resolution.packageManager: yarn@4.17.0), enabled Corepack in CI, useyarn install --immutable, require Node>=20, and updated CI (checkout/cache v4; unit matrix Node 20/22/24/26).get-intrinsic@1.3.0for Gatsby v2 on Node 20+ and enabled Yarn’s supply‑chain age gate by removing thenpmMinimalAgeGateoverride.size-limitworkflow (action not allow‑listed) to prevent CI startup failures; ran Prettier to normalize formatting.Bug Fixes
linkifyhashtag linking by callinginit()after registering thementionandhashtagplugins.StreamUser<UT, AT, CT, RT, CRT, PT>generics inStreamApp/FeedManager; refinedsrc/utils/immutable.tshelpers (narrowedgetInToJSfallback type and preserved explicit nulls intypedUpdater).FeedManagerby guardingfindIndexresults beforeList.remove()in reaction-removal paths.--no-sandboxforChromiumHeadless, setNODE_OPTIONS=--openssl-legacy-providerfor the docs build, replaced the globalmainFieldsoverride with targetedresolve.aliasforlinkifyjsandlinkify-plugin-mention, and addedyarn buildbefore browser bundle and docs builds.Written for commit 17dca6e. Summary will update on new commits.