Ramblings of a Tampa engineer
ini - pull request to [redacted] project.

So for this blog I want to run through my checklist & brain dump when a security pull request comes in. For a little background, the Dependabot project was automating detection of vulnerable dependencies and automatically producing a change to the project to resolve it. It cost money, but then GitHub acquired it and gave it to everyone for free.

So in short - every single project owned had it enabled overnight. Since the ecosystem for child dependencies in both PHP & Node became unmanageable. Lets take the above project "ini". What is it? I sure as hell never installed it directly.

Right off the bat, it lives in the "npm" namespace which is a bit strange. Since it seems strange that npm itself would be hosting a project. We can take a look at the package.json and it wasn't always this way.

"url": "git://github.com/isaacs/ini.git"

So the project lived at isaacs namespace and then moved to npm. I can't find any authoritative source that explains the exact reasons this happens, but there are 322 other repositories under npm so I imagine it happens for crucial dependencies that become slightly abandoned. To prevent the accidental project takeover, abandonment and/or deletion.

So lets figure out why I even have this package. Since if it's unused - lets drop it. Yarn has a helpful command known as "why" that will do just that.

➜  project git:(master) yarn why ini
yarn why v1.22.5
[1/4] Why do we have the module "ini"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "ini@1.3.8"
info Reasons this module exists
   - "laravel-mix#webpack-cli#global-modules#global-prefix" depends on it
   - Hoisted from "laravel-mix#webpack-cli#global-modules#global-prefix#ini"
   - Hoisted from "laravel-mix#webpack-cli#findup-sync#resolve-dir#global-modules#global-prefix#ini"
info Disk size without dependencies: "20KB"
info Disk size with unique dependencies: "20KB"
info Disk size with transitive dependencies: "20KB"
info Number of shared dependencies: 0
Done in 1.05s.

Turns out this dependency exists because of Laravel Mix, which requires Webpack CLI, which requires Global Modules, which requires Global Prefix which requires finally - ini. If that was complicated to read, we can leverage npm for a tree view.

➜  project git:(master) ✗ npm ls ini             
└─┬ laravel-mix@5.0.9
  └─┬ webpack-cli@3.3.12
    ├─┬ findup-sync@3.0.0
    │ └─┬ resolve-dir@1.0.1
    │   └─┬ global-modules@1.0.0
    │     └─┬ global-prefix@1.0.2
    │       └── ini@1.3.8  deduped
    └─┬ global-modules@2.0.0
      └─┬ global-prefix@3.0.0
        └── ini@1.3.8 

So basically this dependency is in use because something 4 layers deep than a direct dependency uses it. A bit scary, but that is the ecosystem we live in. It isn't feasible to audit all code that every dependency uses.

So now we have the security pull request above, which correlates to a low severity prototype pollution.

npm advisory #1589

So we can see I was running version 1.3.5 and now being upgraded to 1.3.8. The advisory will even show me the exact commit that fixed the issue, but I'm not too interested in that. I want to quickly scan the diff between what I have and what the new version is to look for nefarious changes.

Thankfully, GitHub and Dependabot offer a compare view to examine the diff.

ini - v1.3.5 - v1.3.8

This is a lot of changes (9 files, 11,224 additions and 3,650 deletions) to review. Thankfully looking at the diff we see the majority of changes occur in the package-lock.json. A security fix that is also updating its dependencies is slightly alarming, but a quick inspection shows its all dev dependencies.

With that I see tons of changes in regards to CI/automation and linting. This means that most files have spacing changes, but after some scrolling you can spot the security fixes.

Direct commit to fix - ini

So after a quick examination everything looks safe. With the amount of scrutiny put into checking a security upgrade sanctioned by GitHub - it probably is unnecessary. Since I don't exactly do this to every 1058 package I have during a yarn upgrade command.

Thanks to GitHub & Dependabot
You’ve successfully subscribed to Connor Tumbleson
Welcome back! You’ve successfully signed in.
Great! You’ve successfully signed up.
Success! Your email is updated.
Your link has expired
Success! Check your email for magic link to sign-in.