BYOC: add auth token to byoc worker registration#3878
BYOC: add auth token to byoc worker registration#3878ad-astra-video merged 10 commits intolivepeer:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for an auth_token field in BYOC worker capability registration. When a worker registers a capability with an AuthToken, the orchestrator includes a Bearer token in the Authorization header for all subsequent requests to that worker. If the worker returns HTTP 401, the orchestrator removes the capability so the worker can re-register with a valid token.
Changes:
- Adds
AuthToken stringfield toExternalCapabilitywith JSON tagauth_token, and updatesRegisterCapabilityto copy the token on re-registration - Injects
Authorization: Bearer <token>header in all four outbound paths to workers (processJob,StartStream,StopStream,UpdateStream, andmonitorOrchStream) - Adds 401-handling logic in
processJobandStartStreamthat removes the capability on auth failure, with accompanying tests
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
core/external_capabilities.go |
Adds AuthToken field to ExternalCapability struct and copies it during capability re-registration |
byoc/job_orchestrator.go |
Injects auth header into worker requests in processJob; adds 401 handling that removes the capability and returns HTTP 500 |
byoc/stream_orchestrator.go |
Injects auth header in StartStream, StopStream, UpdateStream, and monitorOrchStream; adds 401 removal logic in StartStream |
byoc/job_orchestrator_test.go |
Adds test verifying 401 from worker triggers capability removal and returns 500 to caller |
byoc/stream_test.go |
Adds test verifying 401 from worker triggers capability removal and returns 401 to caller |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3878 +/- ##
===================================================
+ Coverage 32.81005% 32.88163% +0.07158%
===================================================
Files 171 171
Lines 42042 42063 +21
===================================================
+ Hits 13794 13831 +37
+ Misses 27212 27191 -21
- Partials 1036 1041 +5
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
…artStream, StopStream, monitorOrchStream and UpdateStream
…UpdateStream methods
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
eliteprox
left a comment
There was a problem hiding this comment.
Thanks for the PR! This is a good change which helped protect the AI worker service from unauthorized requests.
Strengths:
- Refactoring: createWorkerReq() and processWorkerResp() in stream_orchestrator.go reduce duplication.
- Error handling: 401 responses remove the capability, allowing re-registration.
- Test coverage: Tests cover 401 scenarios in both job and stream flows.
- Security: Uses Bearer token format; token presence is logged without exposing the value.
Suggestions
Please address the Race condition in capability access (medium) comment. The others are small nits.
| return | ||
| } | ||
| // set the headers | ||
| req.Header.Add("Content-Length", r.Header.Get("Content-Length")) |
There was a problem hiding this comment.
Header duplication risk (small nit)
Issue: These headers are set after createWorkerReq(), which already sets at least one header (X-Stream-Id).
Recommendation:
-
Content-Lengthshould be set automatically by Go's HTTP client, worth testing to confirm this works as expected. If so, removeContent-Length(let Go handle it) to ensure no duplication. -
Consider setting all headers within
createWorkerReqfor maintainability.
What does this pull request do? Explain your changes. (required)
Add ability for worker to register with an authentication token similar to ai-runner.
Specific updates (required)
tokenfield parsing in json andAuthTokenfield toExternalCapabilityHow did you test each of these updates (required)
Tested with byoc streaming using the runner-router to apply the auth token
Does this pull request close any open issues?
Checklist:
makeruns successfully./test.shpass