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

feat(taskworker) Add signature based authentication to RPC calls #85533

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

markstory
Copy link
Member

Longer term we may be able to use service mesh authentication, but the requirements for that incur additional infrastructure complexity. This level of authentication will prevent untrusted clients from fetching and updating tasks.

Refs getsentry/taskbroker#57

Longer term we may be able to use service mesh authentication, but the
requirements for that incur additional infrastructure complexity.
This level of authentication will prevent untrusted clients from
fetching and updating tasks.

Refs getsentry/taskbroker#57
@markstory markstory requested a review from a team February 20, 2025 14:51
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Feb 20, 2025
Copy link
Member

@evanh evanh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is an internal-only service why do we need authentication at all?

Copy link

codecov bot commented Feb 20, 2025

Codecov Report

Attention: Patch coverage is 96.77419% with 2 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
tests/sentry/taskworker/test_client.py 93.75% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #85533       +/-   ##
===========================================
+ Coverage   42.27%   87.92%   +45.64%     
===========================================
  Files        9615     9646       +31     
  Lines      544191   546071     +1880     
  Branches    21242    21242               
===========================================
+ Hits       230082   480132   +250050     
+ Misses     313800    65630   -248170     
  Partials      309      309               

@markstory
Copy link
Member Author

If this is an internal-only service why do we need authentication at all?

Should adversaries get access to an internal network, I'd like to avoid any possibilities of ssrf, or malicious requests being made unless the attacker also compromises application containers.

@markstory markstory merged commit 7d17b93 into master Feb 24, 2025
50 checks passed
@markstory markstory deleted the feat-taskworker-request-sign branch February 24, 2025 20:16
@github-actions github-actions bot locked and limited conversation to collaborators Mar 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants