Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update lock file to mitigate critical vulnerability in httpx #1

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

skonapur
Copy link

@skonapur skonapur commented Jul 1, 2024

No description provided.

@skonapur skonapur marked this pull request as ready for review July 1, 2024 19:36
@skonapur skonapur merged commit 0c9f47a into main Jul 1, 2024
skonapur added a commit that referenced this pull request Jul 1, 2024
@skonapur skonapur deleted the security/bump-httpx branch July 1, 2024 19:37
@skonapur skonapur self-assigned this Jul 1, 2024
@sprohaska-vouch
Copy link

So long as it's passing tests, I'm not super concerned about the potential impact of underlying deps changing, especially when weighed against security concerns about leaving these unpatched.

If we do find any issues not handled by the test cases, I'll cut tickets to update tests accordingly. Mode is sort of an odd edge case for the DE team, as we aren't directly responsible for it (mostly) but do have some utils like this floating around.

I'll check out the downstream deps and update here.

@sprohaska-vouch
Copy link

sprohaska-vouch commented Jul 9, 2024

Downstream dependencies surfaced via this GH search:

  • mode-bot: Archived/deprecated. Blast radius nil.
  • tap-anvil: I'm 99% sure this is actually a typo. Doesn't make sense in context + isn't in the pyproject. Blast radius so small it rounds to 0.
  • mode-dashboards: Breaking this would have a severe impact on the analytics team, and prevent their delivery of work product to teams downstream. Blast radius: ⚠️ Moderate, exercise caution.

It's worth noting that the tests I linked above aren't actually running in CI (mostly because there is no CI for this repo). I'm not sure we touch this repo enough to merit building that out now, but... it'd be nice. Noting this in the README and asking PR submitters to run tests before merge would be a good start.

TLDR: If one of us can run the test on our machine, so long as they pass I have no issue with merging this branch back in. Once merged I'll keep an eye out for issues with the dashboard deploy; I'm on call this week so would be a focus for me anyway.

@skonapur
Copy link
Author

Hey @sprohaska-vouch, I tried to run the tests locally. I get a ModuleNotFoundError: No module named 'mode_client'. I feel like I am missing something basic. Can you point me in the right direction?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants