Skip to content

chore(omiGlass): remediate dependabot vulns#10134

Open
undivisible wants to merge 2 commits into
mainfrom
fix/omiglass-deps-bump
Open

chore(omiGlass): remediate dependabot vulns#10134
undivisible wants to merge 2 commits into
mainfrom
fix/omiglass-deps-bump

Conversation

@undivisible

@undivisible undivisible commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Part of #7327. Failure-Class: none

Review in cubic

@Git-on-my-level Git-on-my-level left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking on the omiGlass dependency remediation. I think this needs another pass before merge.

Blocking issues I noticed:

  • @babel/core is declared twice in devDependencies, and also moved into runtime dependencies. Babel is a build-time tool here, so this should stay in devDependencies only and be declared once.
  • The open range >=7.26.0 lets npm resolve Babel 8, and the lockfile currently does resolve @babel/core@8.0.1. Babel 8's package metadata requires Node ^22.18.0 || >=24.11.0, which is a much narrower runtime than the existing React Native/Expo toolchain is likely expecting. If the goal is to remediate the Babel advisory while staying on Babel 7, please use a bounded range such as ^7.26.0 or >=7.26.0 <8 and regenerate the lockfile.
  • Several vulnerability remediations are being forced by adding transitive packages (shell-quote, tmp, undici, form-data, js-yaml, ws) as top-level dev dependencies. That can be acceptable when npm overrides are not available/desired, but this PR should validate that the resulting lockfile still installs and builds omiGlass. Right now the relevant CI jobs are skipped/cancelled, so we do not have a passing install/build signal for this dependency-sensitive change.

Could you please clean up the duplicate/misplaced dependency entries, avoid unintentionally upgrading into Babel 8 / Node 22.18+ requirements, regenerate the lockfile, and include the npm install/build or audit validation you used for omiGlass?

I am marking this for dependency review and tests before maintainer merge consideration.


by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

@Git-on-my-level Git-on-my-level added dependency-review Touches dependencies or lockfiles; needs dependency review needs-tests PR introduces logic that should be covered by tests labels Jul 20, 2026
@undivisible

Copy link
Copy Markdown
Collaborator Author

Kept Babel dev-only and bounded to v7 in ecb9929; the regenerated lockfile passes clean install and audit validation.

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chore: dependabot vuln remediation (omiGlass) — approve only per policy

@undivisible
undivisible force-pushed the fix/omiglass-deps-bump branch from ecb9929 to 10bea1c Compare July 21, 2026 17:17
@skanderkaroui

Copy link
Copy Markdown
Collaborator

Heads up: there is no CI lane for omiGlass, so the green checks here mean nothing ran, not that the upgrade was tested. Worth a quick local build or expo start before merging given package.json changed, not just the lockfile.

@undivisible
undivisible force-pushed the fix/omiglass-deps-bump branch from 10bea1c to 15e933a Compare July 21, 2026 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependency-review Touches dependencies or lockfiles; needs dependency review needs-tests PR introduces logic that should be covered by tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants