fix: an escape hatch for the stream gate until RomM sends the token - #22
Merged
Conversation
The gate 8e3777b added locks out every user of released RomM. Upstream rommapp/romm#3211 is merged and is what people run; its claim response returns the operator's configured host with nothing appended. The RomM half of the gate, which carries the stream token into the iframe URL, is rommapp/romm#3856 and is still open. So the browser loads the stream bare, /verify sees no token, and the document, every asset and the WebSocket upgrade are all refused. Specs STREAM_GATE=off|token, defaulted off until #3856 merges. Deciding enforcement in the broker rather than in the nginx config keeps a mode switch to a restart: as 1736c84 recorded, the nginx patch lands in the writable layer behind its own marker grep, so a gate decided there would need a recreate in each direction. Also records the accepted risk. Defaulted off, :latest serves the interactive desktop with the ROM library mounted again, which the README will state plainly rather than bury.
Four tasks, each with its own test cycle: resolve the mode, bypass enforcement, report which mode is live, then correct the docs that currently promise a gate the default no longer enforces. Records two things the spec could not know. The constant cannot sit beside STREAM_TOKEN_GRACE as specced, because resolving it warns and log does not exist that early in the module, so it goes after the logger. And the seven existing gate tests have to be pinned to STREAM_GATE=token before the bypass lands, or every 403 assertion in them starts passing for the wrong reason under the new default. Container verification is spelled out too, since the suite cannot reach it: the lockout being gone is a 200 from 3001 with no token, and the restart-not-recreate claim is worth checking directly.
Parsing only, nothing enforces it yet. An unrecognized value resolves to 'off' rather than to whatever the default happens to be, because the failure this switch exists to prevent is a stream nobody can reach and a typo must not be able to reintroduce it. Declared after the logger rather than beside STREAM_TOKEN_GRACE with the other stream constants: resolving the value warns, and log does not exist that early in the module.
8e3777b gates the desktop on a token RomM cannot send. rommapp/romm#3211 is merged and is what people run, and its claim response returns the operator's configured host with nothing appended; the half that carries the token is rommapp/romm#3856, still open. So the browser loads the stream bare, /verify sees no token, and the document, every asset and the WebSocket upgrade are all refused. That is a lockout, not a gate, and it is live for everyone on the published image. The decision is made here rather than in the nginx config on purpose. As 1736c84 recorded, that patch lands in the container's writable layer behind its own marker grep, so a gate decided there needs a recreate in each direction; decided in the broker, switching modes is a restart. The token machinery is untouched: /launch still mints, /status still reports, release still clears, and the TTL and grace still tick. Only enforcement is bypassed, so turning the gate back on is one variable. The seven existing gate tests are pinned to STREAM_GATE=token, or every 403 assertion in them would have started passing for the wrong reason under the new default.
.releaserc.json runs semantic-release off main, where feat: cuts a minor version. This branch is a bugfix for a lockout that is live in the published image, so it takes a patch, and the one feat: subject already on the branch has been retitled to match. Tasks 3 and 4 carry their commit messages in the plan text, which the task briefs are extracted from verbatim, so the convention has to change here or the next two implementers would reintroduce it.
|
🎉 This PR is included in version 1.4.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
3 tasks
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
1 participant
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.
Description
The stream gate added in 8e3777b locks out every user of released RomM. This adds the switch that turns enforcement off, defaulted to off until upstream catches up.
The lockout
The gate requires a session token that
POST /launchmints and RomM is supposed to append to the iframe host URL. RomM does not do that yet. rommapp/romm#3211 is merged and is what people are running, and its claim response returns"host": container.get("host", ""), the operator's configured URL with nothing appended. The half that carries the token through is rommapp/romm#3856, still open.So the claim does reach
POST /launchand the broker does mint a token, but the browser then loads the stream host bare. nginx sends the subrequest to/verify,_check_stream_token("")answers "no stream token in the request", and every request is refused: the document, every asset, and the WebSocket upgrade alike. A black stream with nothing on screen to say why, for anyone who pulls:latest.The switch
STREAM_GATE, valuesoffandtoken, defaulting tooff. An unrecognized value warns and resolves tooffrather than tracking the default, because a typo must fail toward a reachable stream and never toward one nobody can open.Enforcement is decided in the broker, not in the nginx config, and that placement is the point. The nginx
auth_requestinjection is untouched and still gates everyserverblock. As 1736c84 recorded, that patch lands in the container's writable layer behind its own marker grep, so a gate decided there would need a--force-recreatein each direction; decided in the broker, it is one environment variable.The token machinery is left alone:
/launchstill mints,/statusstill reports, release still clears, and the TTL and grace window still tick. Only enforcement is bypassed, so turning the gate back on when #3856 merges is one environment variable and adocker compose up -d.Reporting, so the live mode is never a guess
Running with the gate off is a real exposure, so the broker states it rather than leaving it to be deduced. It logs the active mode at startup,
WARNINGwhen off (naming what is exposed and how to close it) andINFOwhen enforcing, andGET /statusreports it asstream_gate.Accepted risk, stated plainly
Defaulted off,
:latestserves the interactive desktop with the ROM library browsable at/files, which is the state before 8e3777b. That is the lesser harm against locking out every user, and the README says so rather than burying it. Keep the container on a network you trust until #3856 ships.Reverting to a gated default is written up in the design doc: change
STREAM_GATE_DEFAULT, update the one test that guards it, update the README, and tell operators on an older RomM to setSTREAM_GATE=off.Review note
A whole-branch review caught one real defect after the feature was otherwise finished, fixed in 563d241. The migration was documented as
docker compose restart, which replays the environment baked in at create time. An operator following it would have setSTREAM_GATE=token, watched the container come back, and believed the desktop was closed while it stayed wide open. It isdocker compose up -d, and no--force-recreateis needed because the changed variable recreates the container on its own and the init re-injects the same gate on the fresh layer.Checklist