Skip to content

fix(client): patch vulnerable dependencies and harden fleet-down redirect - #896

Open
rl-block wants to merge 4 commits into
mainfrom
rongxin/security-dependabot-bumps
Open

fix(client): patch vulnerable dependencies and harden fleet-down redirect#896
rl-block wants to merge 4 commits into
mainfrom
rongxin/security-dependabot-bumps

Conversation

@rl-block

@rl-block rl-block commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Reviewable diff: +21/-16 across 3 files (excludes generated, test, and story files).

Summary

Closes every remaining code-fixable security alert on the repo in one PR: all three open Dependabot alerts (plus the Snyk outdated-dependency alert) via patch-level bumps, and the new CodeQL alert #78 on the fleet-down redirect via a structurally provable same-origin guarantee. Final code PR in the security-alert remediation series after #894 and #895 (both merged); the only remaining work is Security-tab triage of documented false positives, which needs no code.

Notably, the react-router advisory that previously required a v8 major upgrade was backported upstream to 7.18.2, so the planned major-version migration is no longer needed — it collapsed into a routine patch bump.

How it works

Dependency patches. react-router-dom is a direct pinned dependency (package.json + lockfile); js-yaml and dompurify are dev-only transitive dependencies (lockfile only). All three new versions have identical dependency/peer/engine metadata to the versions they replace, so the lockfile diff is exactly the version/resolved/integrity triple per package plus the two react-router pin references. resolved URLs stay on registry.npmjs.org.

  • react-router-dom/react-router 7.18.1 → 7.18.2 (high): RSC-mode CSRF bypass, backported upstream. Also closes the Snyk outdated-dependency alert.
  • js-yaml 4.3.0 → 4.3.1 (high, dev-only): quadratic CPU in !!omap resolution (CVE-2026-59870).
  • dompurify 3.4.12 → 3.4.13 (medium, dev-only): IN_PLACE hook-removal XSS.

Redirect hardening. #894 fixed the open-redirect bypass with a URL-parser origin check plus backslash rejection, but CodeQL re-flagged the sanitized code (alert #78) because its taint tracker cannot recognize custom sanitizer functions. The redirect now prefixes window.location.origin onto the sanitized path, making it structurally impossible for the query parameter to control the scheme or host — the exact shape CodeQL's js/client-side-unvalidated-url-redirection query verifies (it only flags values that can control the start of the URL). The sanitizer stays as defense in depth; navigation behavior is unchanged (absolute same-origin URL instead of relative path).

Diagrams

flowchart LR
    subgraph deps["Dependency patches"]
        A["package.json: react-router-dom 7.18.2"] --> B["lockfile: react-router + react-router-dom 7.18.2"]
        C["lockfile: js-yaml 4.3.1 (dev)"]
        D["lockfile: dompurify 3.4.13 (dev)"]
    end
    subgraph redirect["Fleet-down redirect"]
        E["?from= query param"] --> F["sanitizeRedirectPath: same-origin path or /"]
        F --> G["location.href = location.origin + path"]
    end
    B --> H["npm audit: 0 vulnerabilities"]
    C --> H
    D --> H
    G --> I["CodeQL #78 closes: origin prefix provably controls URL start"]
Loading

Areas of the code involved

Area / package / file What changed Why it matters for review
client/package.json react-router-dom pin 7.18.1 → 7.18.2 Only direct-dependency change
client/package-lock.json version/resolved/integrity for 4 package entries + 2 pin references Verify no unrelated churn (diff is 15 lines)
client/src/protoFleet/utils/fleetDownRedirect.ts Redirect target now origin + sanitized path The security-relevant behavior change; sanitizer unchanged
client/src/protoFleet/utils/fleetDownRedirect.test.ts Expectations updated to origin-prefixed absolute URLs Test-only — confirms same-origin invariant holds for all 13 cases

Key technical decisions & trade-offs

  • Patch bumps only, no npm update tree churn: lockfile entries were updated surgically and validated with a clean npm ci, keeping the diff reviewable and resolved URLs on the public registry.
  • Took 7.18.2 instead of the previously-planned react-router v8 migration: upstream backported the CSRF fix, so the major upgrade (and its route-config/prefetch blast radius) is unnecessary for closing the alert.
  • Origin-prefix over dismissing CodeQL chore(client): enforce ternary for conditional rendering (#33) #78 as a false positive: the sanitized code was already safe, but a shape the analyzer can verify beats a dismissal record — same rationale as the safeInt32 rewrite in fix(sitemap): make safeInt32 clamp provable to static analyzers #895. The redirect becomes an absolute URL, which is behaviorally identical for same-origin navigation.

Testing & validation

  • Clean npm ci from the updated lockfile — integrity verified, npm audit reports 0 vulnerabilities (was 2 high + 1 moderate).
  • tsc --noEmit clean; ESLint clean on changed files; full unit suite: 3,989 tests passed across 385 files, including 13 redirect tests covering external, protocol-relative, backslash, and javascript: payloads.
  • Not covered: Playwright E2E suites were not run locally; CI runs them on the PR. CodeQL's verdict on chore(client): enforce ternary for conditional rendering (#33) #78 lands when analysis re-runs on this PR.

…sories

Three Dependabot alerts, all patch-level bumps in the client tree:

- react-router-dom/react-router 7.18.1 -> 7.18.2 (high): RSC-mode CSRF
  bypass, backported upstream so the previously-required v8 major
  upgrade is no longer needed. Also closes the Snyk outdated-dependency
  alert on the same package.
- js-yaml 4.3.0 -> 4.3.1 (high, dev-only): quadratic CPU in !!omap
  resolution (CVE-2026-59870).
- dompurify 3.4.12 -> 3.4.13 (medium, dev-only): IN_PLACE hook removal
  XSS.

Dependency metadata is unchanged across all three bumps; npm audit now
reports 0 vulnerabilities.
@rl-block
rl-block requested a review from a team as a code owner August 7, 2026 19:47
@github-actions github-actions Bot added dependencies Pull requests that update a dependency file javascript Pull requests that update javascript code client review-policy: needs-review Managed by the Review Policy workflow. labels Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Note: This is an automated security-focused code review generated by Codex.
It should be used as a supplementary check alongside human review.
False positives are possible - use your judgment.

Scope summary

  • Reviewed pull request diff only (7ba4b833e5d9d851611db72902d96cdf80eb4373...3be3231742637db00724851b334e9d527bf115f1, exact PR three-dot diff)
  • Model: gpt-5.6-sol

💡 Click "edited" above to see previous reviews for this PR.


Review Summary

Overall Risk: NONE

Findings

No security, correctness, or reliability issues were identified in the changed hunks.

Notes

The redirect remains same-origin under URL parser edge cases. The firmware E2E relaxation is followed by a deterministic terminal-state assertion, and the dependency lockfile is consistent. Project tests were not run because dependencies are not installed in the review workspace.


Generated by Codex Security Review |
Triggered by: @rl-block |
Review workflow run

Prefix window.location.origin on the redirect target so the query
parameter structurally cannot control the scheme or host. CodeQL's
js/client-side-unvalidated-url-redirection query (alert #78) flags
values that can control the start of the URL and does not recognize
the custom sanitizer; the origin prefix is a shape it can verify.
The sanitizer stays as defense in depth. Navigation behavior is
unchanged (absolute same-origin URL instead of relative path).
@rl-block rl-block changed the title fix(client): patch react-router, js-yaml, and dompurify security advisories fix(client): close remaining security alerts — dependency patches and provable redirect hardening Aug 7, 2026
@rl-block rl-block changed the title fix(client): close remaining security alerts — dependency patches and provable redirect hardening fix(client): patch vulnerable dependencies and harden fleet-down redirect Aug 7, 2026
The firmware install spec reloads the page and asserts the status
widget shows the transient "Installing" state. When the simulated
install completes during the reload window, the widget already shows
"Reboot required" and the assertion polls for a state that never
returns (failed twice in CI on this race). Accept either state; the
following test step already validates the reboot-required UI
deterministically after waiting for the installed status via the API.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

client dependencies Pull requests that update a dependency file javascript Pull requests that update javascript code review-policy: needs-review Managed by the Review Policy workflow.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant