kethinov / no-cli-ads

🛑 [NO LONGER MAINTAINED] Ad blocker for command line interface ads.
Other
399 stars 6 forks source link

Other version based on directly patching npm #3

Open mkg20001 opened 5 years ago

mkg20001 commented 5 years ago

I've made https://github.com/mkg20001/npm-adblock

It's basically the same, except it works by directly patching npm to only run postinstall hooks if they aren't blacklisted by the criteria outlined in https://github.com/mkg20001/npm-adblock/blob/master/index.js

That one should work on windows (should, but might not) It's a bit better since it is always enabled after installation and can even re-patch itself after npm self-updates (But updates via the package manager break it)

What do you think?

kethinov commented 5 years ago

What if I want to run the postinstall scripts for the other modules that were installed? Can it blacklist a single postinstall script but run the others?

mkg20001 commented 5 years ago

It only blacklists the postinstall for modules that were explicitly added to the blacklist in https://github.com/mkg20001/npm-adblock/blob/master/index.js It matches either by module name or by strings in the command. for example when the hook command starts with "opencollective-postinstall" it gets blacklisted regardless of the module name. Should it ever break a module, because it's post install does more than display ads, then other workarrounds will be implemented to get just the ads blocked.

mkg20001 commented 5 years ago

The blacklist currently is written by me as js code (and updated with the app), but if it were to grow bigger I might also serve a json file with patterns and let the client download it, similar to current adblockers.

adrianmcli commented 5 years ago

@mkg20001 I like your approach a lot better actually. The code is also a lot more straight-forward.

kethinov commented 5 years ago

Assuming I'm reading all this correctly, it looks like there are some pros and cons.

Pros:

Cons:

mkg20001 commented 5 years ago

Probably(?) doesn't allow partial execution of a postinstall script

The postinstall can be modified in the filterHook function

Alters npm itself, which is more invasive than a passive monitoring process.

Yes and no. My Projects folder is huge. When I'd use your tool... it would hit some limit. That's why I made my version.

The second reason why I made the other tool, is because some modules have the ads as a node -e "console.log(...)" postinstall command. Since npm only writes the package.json to fs and doesn't read it again during the same installation, it can't be blocked via fs.

For example parcel does this with their ad.

kethinov commented 5 years ago

Ah yeah, that's a good point. If you can modify the postinstall script using this method but still allow it to run, then the only distinction of relevance is whether a monitoring process feels ickier to you or modding npm itself does, which is somewhat subjective.

mkg20001 commented 5 years ago

then the only distinction of relevance is whether a monitoring process feels ickier to you or modding npm itself does, which is somewhat subjective.

Would be curious to see how something like this parcel ad can be blocked.

Checking /proc/PID/cmdline and killing it would be an option. But that way we can't get sure we kill it at the right time.

kethinov commented 5 years ago

My monitoring process approach could catch it. Would need to alter watcher.js to scan for parcel being installed, then once it is alter the contents of its package.json at that line.

mkg20001 commented 5 years ago

Would need to alter watcher.js to scan for parcel being installed, then once it is alter the contents of its package.json at that line.

Does npm really re-read the package.json during the same install? I doubt that

kethinov commented 5 years ago

Good question. This would have to be tested.

kethinov commented 5 years ago

Filed an issue for that. Also crosslinked your repo from this one's README.

mkg20001 commented 5 years ago

@kethinov I'm still curious to see if npm reloads the package.json. I looked through the npm code, and it only seems to write it if I'm not mistaken.

The only occurrence for readPackageJSON in install is lib/install.js:603 where it reads only the top package.json

  readPackageJson(path.join(this.where, 'package.json'), log, false, (err, data) => {
    if (err) return cb()
    this.currentTree = createNode({
      isTop: true,
      package: data,
      path: this.where
    })
    doOneAction('preinstall', this.where, this.currentTree, log.newGroup('preinstall:.'), cb)
  })