Repository navigation
Conversation
…er's token With a broadcast, the coordinator sends its admission key to every stream source, and each refuses GET /frames (403, before the upgrade) without a direct token made with it. A late viewer holds no token, so flipping its page to direct opens nothing. - Renderer (fleet/renderer src/admit.cpp): POST /scene takes admit_keys (hex, at most 4). The pipeline keeps the newest and the ones before them, so a token made just before a new key still works; a scene without keys turns the check off. /frames checks admit= with HMAC-SHA256 and CRYPTO_memcmp before upgrading. - Scenes: SharedScene and RendererScene carry admit_keys (never in a snapshot). The coordinator sets them on every scene (Cmd::AdmitKeys, Handle::set_admit_keys) only when it runs a broadcast; agents and EC2 tiles post them with each scene. - Aggregators: Layout carries admit_keys. The public listener's /frames needs a token; the aggregator's own tile subscriptions carry one it makes. The agent passes its scene's keys; the coordinator its own to the EC2 and broadcast aggregators. The poster's subscriptions carry a token. Tests, each observed failing without its check: with_admission_keys_frames_need_a_direct_token_and_tiles_get_one (test_fleet_aggregate; a request without a token was served), the renderer's host tests (a cross-language token vector shared with admission.rs, expiry, other keys, malformed tokens, key rotation, admit_keys in a scene) and a source-level check that /frames asks the pipeline before it upgrades (failed with the check removed), and admission_keys_reach_renderers_and_layouts_but_no_browser (test_fleet_coord). Fleet suites 206 passed; renderer host tests 188 passed; clippy clean. pipeline.cpp and server.cpp pass g++ -fsyntax-only here; the renderer image needs CUDA and is built by the AMI pipeline.
Owner
Author
|
@codex review |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This branch has not been deployed
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
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.
Stacked on: feat/broadcast-page (PR #1396)
Server-side enforcement of the broadcast's admission: with a broadcast, renderers and aggregators refuse
GET /frames(403, before the upgrade) without a direct token made with the coordinator's key. A late viewer holds no token, so flipping its page into direct mode opens nothing.Contract
fleet/renderer/src/admit.cpp):POST /scenetakesadmit_keys(hex, at most 4). The pipeline keeps the newest and the ones before them, so a token made just before a new key still opens a stream; a scene without keys turns the check off./frameschecksadmit=(HMAC-SHA256, constant-time compare, expiry) before it upgrades.SharedSceneandRendererScenecarryadmit_keys; they never reach a browser. The coordinator sets them (Handle::set_admit_keys) only when it runs a broadcast, so a fleet without one checks nothing. Agents and EC2 tiles post them with every scene.Layout.admit_keys. The public (TLS) listener's/framesneeds a token; the aggregator's own tile subscriptions carry one it makes. The agent passes its scene's keys; the coordinator its own to the EC2 and broadcast aggregators. The poster's subscriptions carry a token.Downstream impact
Takes effect for renderers only with a renderer image (host and tile AMIs) built from a commit with this change, and only once the coordinator runs with
--broadcast-listen. Until then the scene field is ignored by older renderers. The page already sendsadmit=(PR #1396), so seated viewers keep their streams when enforcement turns on.Known limit, unchanged by this PR:
POST /sceneanswers anyone who reaches a renderer and sends a JSON content type, so a hostile client could replace the keys. That is the scene route's own exposure (it can replace the scene too), tracked separately.Evidence
with_admission_keys_frames_need_a_direct_token_and_tiles_get_one(test_fleet_aggregate): the tile subscription carries a token the key validates; the public listener refuses no token and a forged one with 403 and serves a valid one. It failed with the old route (a request without a token was served).make test-host, 188 checks): a token vector shared withadmission.rs(key07x32, viewerv-3, expiry 1000), expiry, another key, the previous key, malformed tokens, bad hex keys, key rotation,admit_keysin a scene; and a source-level check that/framesasks the pipeline before it upgrades, which failed with the check removed.pipeline.cppandserver.cpppassg++ -fsyntax-onlyhere (the image needs CUDA).admission_keys_reach_renderers_and_layouts_but_no_browser(test_fleet_coord): the agent's scene and the renderer body carry the key, the broadcast layout carries it, the snapshot text does not.make clippyclean.