3 ms·

Instead of changing the module's URL to e.g. a private evil GitHub repository, how about registering a new package within the official npm registry, using homog

by mxscho 7y ago

Instead of changing the module's URL to e.g. a private evil GitHub repository, how about registering a new package within the official npm registry, using homoglyphs/similar-looking characters. For diff viewers that are highlighting entire lines for changed content, the difference in the package name wouldn't be spotted in reviews when also changing the version or some other part of the URL. Or am I missing something?

saghm 7y ago
At least when I review code, if I see a line changed that doesn't need to be, I leave a comment asking why and hold off on approving the code for merge until I figure out what's going on. This is more due to not having changes people made by mistake merged in rather than a strategy for dealing with malicious submitters, though.
- iudqnolq 7y ago
  I think the issue is that the version number and name change would be the same line. Maybe you'd see that the line changed and that the version number changed and assume that was the only change for the line. (Depending on client, might not highlight specific sub-line changes)
- jakear 7y ago
  Curious, would you ever approve a third party change to a lockfile? I maintain a very large OSS project and I don’t know any reason I would accept a PR that changes the lockfile.
  - reificator 7y ago
    Would you accept a PR that changes the package.json file to add a new dependency?
    - jakear 7y ago
      Not likely, we try to minimize dependancies as much as possible. If a new feature were to absolutely necessitate a new dependency, it probably wouldn't be open for third party contributions in the first place (not least of all because maintenance of the dep now falls onto us if ever upstream falters).
    - tobyhinloopen 7y ago
      You could just discard the PR’s lock and generate your own in the PR
styfle 7y ago
I believe npm blocks publishing packages with similar names to existing packages. https://stackoverflow.com/a/54135900/266535 https://stackoverflow.com/a/54135900/266535

by mxscho 7y ago

saghm 7y ago

At least when I review code, if I see a line changed that doesn't need to be, I leave a comment asking why and hold off on approving the code for merge until I figure out what's going on. This is more due to not having changes people made by mistake merged in rather than a strategy for dealing with malicious submitters, though.

iudqnolq 7y ago
I think the issue is that the version number and name change would be the same line. Maybe you'd see that the line changed and that the version number changed and assume that was the only change for the line. (Depending on client, might not highlight specific sub-line changes)
jakear 7y ago
Curious, would you ever approve a third party change to a lockfile? I maintain a very large OSS project and I don’t know any reason I would accept a PR that changes the lockfile.
- reificator 7y ago
  Would you accept a PR that changes the package.json file to add a new dependency?
  - jakear 7y ago
    Not likely, we try to minimize dependancies as much as possible. If a new feature were to absolutely necessitate a new dependency, it probably wouldn't be open for third party contributions in the first place (not least of all because maintenance of the dep now falls onto us if ever upstream falters).
  - tobyhinloopen 7y ago
    You could just discard the PR’s lock and generate your own in the PR

styfle 7y ago

I believe npm blocks publishing packages with similar names to existing packages. https://stackoverflow.com/a/54135900/266535 https://stackoverflow.com/a/54135900/266535