privacy-tech-lab / privacy-pioneer

Privacy browser extension for analyzing web traffic of visited websites
https://www.privacytechlab.org/
Other
22 stars 1 forks source link

Bump follow-redirects from 1.15.2 to 1.15.4 #557

Closed dependabot[bot] closed 4 months ago

dependabot[bot] commented 5 months ago

Bumps follow-redirects from 1.15.2 to 1.15.4.

Commits
  • 6585820 Release version 1.15.4 of the npm package.
  • 7a6567e Disallow bracketed hostnames.
  • 05629af Prefer native URL instead of deprecated url.parse.
  • 1cba8e8 Prefer native URL instead of legacy url.resolve.
  • 72bc2a4 Simplify _processResponse error handling.
  • 3d42aec Add bracket tests.
  • bcbb096 Do not directly set Error properties.
  • 192dbe7 Release version 1.15.3 of the npm package.
  • bd8c81e Fix resource leak on destroy.
  • 9c728c3 Split linting and testing.
  • Additional commits viewable in compare view


Dependabot compatibility score

You can trigger a rebase of this PR by commenting @dependabot rebase.


Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/privacy-tech-lab/privacy-pioneer/network/alerts).

Note Automatic rebases have been disabled on this pull request as it has been open for over 30 days.

SebastianZimmeck commented 5 months ago

@natelevinson10, take a look and see why the check fails and how we could fix that. This may be a bit too much in the weeds since you did not have a lot of time understanding the extension. But @jjeancharles can assist, and we can discuss here or in the chat any questions you may have.

SebastianZimmeck commented 5 months ago

Related Dependabot alert.

natelevinson10 commented 5 months ago

I’m not too sure how to get started with figuring out what caused the tests to fail, I looked at the error logs which indicate that Jest is failing to parse holdAPI.js on line 2, column 0. I don't see a holdAPI.js file in the repo, but I remember creating one and manually inputting the ipinfo.io API key into it. Any good idea of how to approach this failed check?

SebastianZimmeck commented 5 months ago

@jjeancharles, do you have a suggestion?

SebastianZimmeck commented 5 months ago

@JoeChampeau will also help looking into this, especially, for what are we using this library. As @danielgoldelman mentioned, it may be indirectly, i.e., we use a library that uses this library. Maybe, that needs updating. Maybe, we are also not using the library in the first place. In any case, some more digging would be good.

JoeChampeau commented 5 months ago

The common issue among all these Dependabot PRs failing the checks seems to be that the command which generates an API token for testing relies on using a GitHub Actions secret that Dependabot doesn't have access to (secrets.APIIPTOKEN). Apparently this wasn't always the case, and the change was made a couple years back.

I'll do more research into the issue, but the key takeaways are 1. there are no issues with the updated dependency, so we're free to merge and 2. we might want to consider giving Dependabot access to the IP token, although there may or may not be vulnerabilities associated with that (there's a reason why the change was made).

In any case, we always have recourse to simply recommitting like I did, which made me the author and, therefore, bypassed the issue with Dependabot's access perms.

SebastianZimmeck commented 5 months ago

Great work, @JoeChampeau!

SebastianZimmeck commented 5 months ago

Depending on how we fix that or not, maybe, write that up briefly in the Known Issues section of the readme.

JoeChampeau commented 4 months ago

Alright, I've done a good amount of research on this issue, and there appears to be no clear consensus. Copying the IPinfo key to Dependabot's personal secrets registry (which is secured and encrypted like normal Actions secrets) suffers only from the possibility that a malicious update could be pushed to one of PP's dependencies which leverages Dependabot's access to the secret to steal it during the build tests for the automated PR. My assessment is as follows:

  1. We could give Dependabot access to this key. This security issue seems remote, given the fact that, from what I can tell, we generally use pretty mainstream packages that probably won't have people publishing malicious code to them. Furthermore, while we'd have to replace the IPinfo key if it happened to be stolen, it's not exactly the most sensitive information.
  2. We could maintain the current system and update the readme accordingly, where we'd simply have to recommit Dependabot branches to run the tests with access to the secrets. This would give us the opportunity to vet the new version of the dependency manually before running the tests. We'd have to actually do that before re-committing though, otherwise it'd have the same security issues as Option 1.

What do you think @SebastianZimmeck?

SebastianZimmeck commented 4 months ago

OK, good points, @JoeChampeau! I'd say, let's go with the first option. We could disable the key quickly if worst comes to worst.

JoeChampeau commented 4 months ago

Sounds good! I'm checking now and it looks like I don't have access to the Settings tab for this repo. I think it might be accessible only to you, @SebastianZimmeck. It should be a pretty straightforward process, though, you just have to navigate to Settings -> Security -> Secrets and Variables -> Dependabot and add a new one named APIIPTOKEN with the value of the IPinfo token.

SebastianZimmeck commented 4 months ago

OK, these are good instructions, @JoeChampeau. I did as you said.

JoeChampeau commented 4 months ago

Great! Closing and merging for now, but we'll know for sure it's worked once the next Dependabot PR comes.