Skip to content

fix(webapp): patch prod-dep vulnerabilities + build on modern Node - #7

Closed
hstern wants to merge 2 commits into
hhru:masterfrom
hstern:fix/webapp-prod-dep-vulns
Closed

fix(webapp): patch prod-dep vulnerabilities + build on modern Node#7
hstern wants to merge 2 commits into
hhru:masterfrom
hstern:fix/webapp-prod-dep-vulns

Conversation

@hstern

@hstern hstern commented Jun 3, 2026

Copy link
Copy Markdown

Two related fixes so the webapp is secure and builds on a current toolchain.

1. Production-dependency vulnerabilities

npm audit --omit=dev reported 2 critical + 7 moderate. Cleared both criticals (plus 2 moderates) via npm overrides + direct bumps, without the breaking mattermost-redux 5→11 major:

  • overrides: form-data ^3.0.4 (was 3.0.0) — CRITICAL (unsafe boundary RNG); also clears the mattermost-redux "critical", which was flagged only for inheriting it. moment-timezone ^0.5.35 (was 0.5.32) — command injection.
  • @babel/runtime ^7.29.7 (was 7.11.2) — direct dep, bumped directly (npm refuses an override on a direct dep).
  • react 16.14.0 (was 16.13.1) — satisfies the transitive react-dom@16.14.0 peer (also the npm install ERESOLVE blocker).

Result: 2 critical + 7 moderate → 0 critical/high, 6 moderate. The remaining 6 (uuid, nanoid, redux-devtools-core, remote-redux-devtools, socketcluster-client) only patch via breaking majors under mattermost-redux@5.33.1 and are low real-risk (edge-case CVEs / dev-tooling).

2. Build on modern Node (no manual flag)

webpack 4 + terser-webpack-plugin hash with md4, which OpenSSL 3 (Node 17+) removed → ERR_OSSL_EVP_UNSUPPORTED, so the build broke on current Node unless run with NODE_OPTIONS=--openssl-legacy-provider. Baked that into the webpack npm scripts via cross-env so npm run build / make dist just work on Node 17+ (and still on older Node), no manual env.

Verification

  • npm run check-types — passes (no type regressions).
  • bare npm run build — succeeds on Node 26 (no manual NODE_OPTIONS).
  • make dist — produces the plugin bundle; server compiles for all 5 targets on Go 1.26.

Not included (deferred)

  • The remaining 6 moderate vulns and the full toolchain refresh (webpack 5, etc.) really want a sync with upstream mattermost/mattermost-plugin-github.
  • install-go-tools pins golangci-lint@v1.59.1, which won't compile on Go 1.23+. Bumping it (e.g. v1.64.8) compiles on modern Go but surfaces pre-existing lint findings that would fail make check-style — left out of this PR to avoid restyling code / a CI regression; worth pairing with the upstream sync.

🤖 Generated with Claude Code

npm overrides pin the vulnerable transitive deps to patched in-range versions,
plus direct bumps of @babel/runtime and react. Clears BOTH critical advisories
(form-data unsafe boundary RNG; mattermost-redux only inherited it) without the
breaking mattermost-redux 5->11 major bump.

  * overrides: form-data ^3.0.4 (was 3.0.0, CRITICAL); moment-timezone ^0.5.35
    (was 0.5.32, command injection)
  * @babel/runtime ^7.29.7 (was 7.11.2, RegExp-complexity) -- direct dep, so
    bumped directly (npm refuses an override on a direct dep)
  * react 16.14.0 (was 16.13.1) to satisfy the transitive react-dom@16.14.0 peer

npm audit --omit=dev: 2 critical + 7 moderate -> 0 critical/high, 6 moderate.
The remaining moderates (uuid, nanoid, redux-devtools-core, remote-redux-devtools,
socketcluster-client) only patch via breaking majors under mattermost-redux
5.33.1 and are low real-risk (edge-case CVEs / dev-tooling) -- deferred to the
eventual upstream sync with mattermost-plugin-github.

Verified: npm run check-types passes; npm run build succeeds (with
NODE_OPTIONS=--openssl-legacy-provider, required for webpack 4 on Node 17+).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hstern
hstern requested a review from SCREEN88 as a code owner June 3, 2026 15:25
webpack 4 (and terser-webpack-plugin) hash with md4, which OpenSSL 3 (Node 17+)
removed -> ERR_OSSL_EVP_UNSUPPORTED, so `npm run build` failed on current Node
unless run with NODE_OPTIONS=--openssl-legacy-provider. Bake that into the
webpack npm scripts via cross-env so `npm run build` / `make dist` just work on
Node 17+ (and still on older Node), no manual env needed.

Verified: bare `npm run build` succeeds on Node 26; `make dist` produces the
plugin bundle.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hstern hstern changed the title fix(webapp): patch production dependency vulnerabilities (clears both criticals) fix(webapp): patch prod-dep vulnerabilities + build on modern Node Jun 3, 2026
@SCREEN88

SCREEN88 commented Jun 5, 2026

Copy link
Copy Markdown

Hi, thanks for the PR. I think the right approach here would be to pull the master branch up to upstream. I've already started doing that — I should be able to continue in about a week and a half, and then test everything thoroughly.

@hstern

hstern commented Jun 6, 2026

Copy link
Copy Markdown
Author

Superseded by #10, which does the upstream resync you suggested ("pull the master branch up to upstream") rather than patching the old base. The two fixes here fold into riding current upstream: the modern toolchain already builds on current Node and pins golangci-lint v2, so the prod-dep bumps and the --openssl-legacy-provider workaround are no longer needed.

Closing in favor of #10. Thanks for the steer.

@hstern hstern closed this Jun 6, 2026
@hstern

hstern commented Jun 6, 2026

Copy link
Copy Markdown
Author

For the record (Issues are disabled on this repo, so noting them here) — three pre-existing bugs surfaced while live-testing the resync in #10. They're present in 2.3.0, not introduced by the resync, and I'm happy to fix each in a follow-up once #10 lands:

  1. issues webhook events never post. eventTypeMapping (server/plugin/webhook.go) has no "issues" entry, so issue open/close/edit events fall through to go-github's ParseWebHook, which fails on the Forgejo payload — json: cannot unmarshal string into Go struct field Repository.issue.repository.owner of type github.User. The case *github.IssuesEvent is never reached. Fix: add an FIssueEvent struct and map "issues" to it, like the other F* handlers.

  2. Rotated OAuth refresh tokens aren't persisted. Forgejo rotates the refresh token on use; the plugin doesn't store the rotated token, so after the access token expires (or the plugin restarts) the next refresh fails with oauth2: "unauthorized_client" "token was already used", breaking per-user API calls (private-repo permission checks, sidebar, todo) until /forgejo disconnect + /forgejo connect. Fix: wrap the oauth2.TokenSource to persist refreshed tokens back to the KV store.

  3. makeForgejoRequest nil-panics. When forgejoClient.Get() errors the response is nil, but the code proceeds to json.NewDecoder(response.Body) → nil-pointer dereference (recovered by withRecovery, but the request fails). Reliably triggered by HH-258530 sync with upstream and fix tests #2. Fix: return the zero value on error / nil response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants