switch backend to listen on free port and require auth#35
Merged
Ofir Gluzman (ofirgluzman) merged 1 commit intomainfrom Mar 9, 2026
Merged
switch backend to listen on free port and require auth#35Ofir Gluzman (ofirgluzman) merged 1 commit intomainfrom
Ofir Gluzman (ofirgluzman) merged 1 commit intomainfrom
Conversation
Member
Alex Gershovich (alexger)
commented
Mar 7, 2026
Alex Gershovich (alexger)
commented
- instead of hard-coded port we dynamically pick a free one
- backend<->frontend channel is guarded by a random auth token
| auth_header = request.headers.get("authorization", "") | ||
| if auth_header.startswith("Bearer ") and _token_matches(auth_header[7:]): | ||
| return await call_next(request) | ||
| if auth_header.startswith("Basic "): |
There was a problem hiding this comment.
why needed in addition to bearer token?
Member
Author
There was a problem hiding this comment.
it isn't but curl is slightly more convenient to use with basic: curl -u ':<token'> ...
| ) -> StarletteResponse: | ||
| if not auth_token: | ||
| return await call_next(request) | ||
| if request.method == "OPTIONS": |
There was a problem hiding this comment.
Is this under assumption the options requests get swallowed by the cors middleware? If so, worth adding a comment
Member
Author
There was a problem hiding this comment.
OPTIONS mustn't require auth, so the code lets it pass. There is no assumption cors middleware exists at all...
backend/ltx2_server.py
Outdated
| if os.environ.get("LTX_NO_AUTH", "").lower() in ("1", "true", "yes"): | ||
| auth_token = "" | ||
| else: | ||
| auth_token = os.environ.get("LTX_AUTH_TOKEN", "") or secrets.token_urlsafe(32) |
Member
There was a problem hiding this comment.
Suggestion:
- Server just gets LTX_AUTH_TOKEN as optional token, null means no token
- Electron main is the one generating the token and starts the backend with LTX_AUTH_TOKEN populated to it
- no LTX_NO_AUTH, this is covered by LTX_AUTH_TOKEN=ll
- No self generation of the token by the backend (secrets.token_urlsafe(32)) and picking it from the log by Eelectron
Pros
- Simpler: just one token source
- More secured: token is not printed in the std logs (which I also just started to route to the log file before reading this PR at 90259a9)
- No log regex parsing communication
- instead of hard-coded port we dynamically pick a free one - backend<->frontend channel is guarded by a random auth token
Alex Gershovich (alexger)
force-pushed
the
feature/fix-port-conflict
branch
from
54bff38 to
c6068da
Compare
Member
Author
|
done, i've moved the token selection into electron as you suggested |
Ofir Gluzman (ofirgluzman)
approved these changes
Mar 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.