feat(auth): allow non-admin user role to use HTTP sink/IO nodes#634
Conversation
The built-in user role excluded transport::http::* entirely and all streamkit::* nodes, forcing HTTP-transport gateways to run with an admin token. Allow the safe sink/IO nodes (transport::http::mse, streamkit::http_input, streamkit::http_output) while keeping transport::http::fetcher denied (SSRF risk), and document a least-privilege gateway role. Closes #632 Signed-off-by: streamkit-devin <devin@streamkit.dev>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![staging-devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/870844?s=60&v=4){kind=small}
| "streamkit::http_input".to_string(), | ||
| "streamkit::http_output".to_string(), |
There was a problem hiding this comment.
📝 Info: http_input/http_output are implicitly allowed for oneshot regardless of allowlist
Adding streamkit::http_input and streamkit::http_output to the user role's allowed_nodes is harmless but redundant for the oneshot path: apps/skit/src/server/oneshot.rs:529-536 explicitly skips these marker kinds, treating them as implicitly allowed whenever oneshot execution is permitted. The allowlist entries only have an effect at the other is_node_allowed call sites (sessions/websocket/mcp/validation), where these oneshot-only marker nodes would not normally appear. Net: the entries are safe and the documented gateway example is still correct, just over-specified for oneshot use.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Thanks — agreed, info-only and correct. The oneshot path (server/oneshot.rs) skips these marker kinds so they're implicitly allowed there; the allowlist entries only matter at the session/websocket/mcp/validation is_node_allowed call sites. I'm keeping them for consistency with the documented gateway role example (and the new samples/skit.toml role) so the same allowlist works whether or not a node is gated — over-specified but not incorrect.
Tighten the sample user role to deny transport::http::fetcher (was allowed via the transport::* wildcard) and add a least-privilege gateway role example. Signed-off-by: streamkit-devin <devin@streamkit.dev>
| ] | ||
|
|
||
| # Only the specific plugin(s) the gateway needs (example: servo for web-capture). | ||
| allowed_plugins = ["plugin::native::servo"] |
There was a problem hiding this comment.
🚩 Gateway example references plugin::native::servo
Both the docs (docs/src/content/docs/guides/authorization.md:138) and sample config (samples/skit.toml:535) use plugin::native::servo as the gateway's allowed plugin for the web-capture example. Reviewer may want to confirm a servo native plugin kind actually exists/is the right name used by the web-capture example, otherwise the example would be non-functional as written.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Confirmed valid — plugin::native::servo is the correct kind. The servo plugin (plugins/native/servo/plugin.yml) declares kind: native, node_kind: servo, and the existing sample pipelines (samples/pipelines/oneshot/web_capture.yml, web_pip_compositor.yml) use kind: plugin::native::servo. So the gateway example is functional as written.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #634 +/- ##
==========================================
- Coverage 85.01% 85.01% -0.01%
==========================================
Files 249 249
Lines 74801 74818 +17
Branches 2437 2437
==========================================
+ Hits 63595 63609 +14
- Misses 11200 11203 +3
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Mirror Permissions::user() in samples/skit.toml: drop blanket core::* (which exposed core::file_writer) for the safe core subset, add video::* and plugin usage, keeping transport::http::fetcher denied. Extend the sample config test to assert the user/gateway node allowlists. Signed-off-by: streamkit-devin <devin@streamkit.dev>
| allowed_nodes = [ | ||
| "transport::http::mse", # serve live casts to the browser (MSE) | ||
| "streamkit::http_input", # oneshot request body | ||
| "streamkit::http_output", # oneshot response | ||
| "core::*", | ||
| ] |
There was a problem hiding this comment.
📝 Info: Gateway role's core::* allows arbitrary-write file_writer despite least-privilege framing
The new gateway role in samples/skit.toml:538-543 and the docs example (docs/src/content/docs/guides/authorization.md:132-137) grant core::*, which includes core::file_writer. The built-in user role (apps/skit/src/permissions.rs:172-181) deliberately enumerates individual core nodes and excludes core::file_writer because of arbitrary-write risk. So the role labelled "least-privilege" is actually broader (for core nodes) than the user role with respect to file_writer. This is a config/security-policy choice rather than a code bug, and security findings are out of scope for this review, but the inconsistency with the stated least-privilege intent may be worth a deliberate decision.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Fixed: the gateway example (sample config + docs) now grants only the safe core plumbing nodes (passthrough, file_reader, pacer, sink) instead of core::*, so it no longer exposes core::file_writer — consistent with the least-privilege intent and the built-in user role.
| "user/*.yaml", | ||
| ] |
There was a problem hiding this comment.
📝 Info: Sample user role allowed_samples diverges slightly from code default
The samples/skit.toml user role's allowed_samples includes demo/*.yml/demo/*.yaml (lines 480-481), while the code default Permissions::user() (apps/skit/src/permissions.rs:149-157) does not. This is a pre-existing divergence not introduced by this PR (the PR only touched allowed_nodes/allowed_plugins for the user role), so it is not a regression, but the code and sample are not a perfect mirror of each other despite the PR's stated goal of mirroring Permissions::user().
(Refers to lines 475-484)
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Fixed: removed demo/* from the sample user role's allowed_samples so it now mirrors the built-in Permissions::user() (oneshot/dynamic/user only).
Drop core::* from the gateway example (it exposed core::file_writer, contradicting the least-privilege intent) for the safe core plumbing subset, and remove demo/* from the sample user role's allowed_samples to match the built-in Permissions::user(). Signed-off-by: streamkit-devin <devin@streamkit.dev>
The gateway example could never run its own pipeline: is_node_allowed() runs before the plugin check, so plugin::native::servo (absent from allowed_nodes) was rejected, and the role lacked the video/containers nodes the servo->encode->mux->serve pipeline needs. Grant exactly those kinds. Also align the sample user role with Permissions::user(): add the list/read/write/delete_samples flags (without which allowed_samples is unreachable) and core::param_bridge (a safe in-graph node). Drop the streamkit::http_input/http_output allowlist entries from both the built-in and sample roles - they are oneshot-only markers never gated by is_node_allowed, so listing them was dead config. Signed-off-by: streamkit-devin <devin@streamkit.dev>
| # Sample management (mirrors the built-in Permissions::user()); without these | ||
| # flags the allowed_samples list below would be unreachable. | ||
| list_samples = true | ||
| read_samples = true | ||
| write_samples = true | ||
| delete_samples = true |
There was a problem hiding this comment.
📝 Info: TOML user role now aligned with code, fixing previously-unreachable sample allowlist
Before this PR the samples/skit.toml user role omitted list_samples/read_samples/write_samples/delete_samples, which default to false. Because a TOML-defined role fully replaces the built-in Permissions::user(), the allowed_samples list was effectively unreachable for the sample config's user role. Adding these flags (samples/skit.toml:476-479) corrects that and brings the TOML in line with the code defaults in apps/skit/src/permissions.rs. This is a real fix, not a regression.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Correct — this was the fix for the second validation finding. Without list/read/write/delete_samples the allowed_samples list was dead config in the sample's user role. Now the sample matches Permissions::user() and the list is reachable.
| "user/*.yml", | ||
| "user/*.yaml", | ||
| ] | ||
|
|
||
| # Users can use most nodes except potentially dangerous ones | ||
| # Users can use most nodes except potentially dangerous ones. Mirrors the | ||
| # built-in Permissions::user() default. Allowed: MoQ + the HTTP `mse` sink | ||
| # (serves over the caller's own request). Denied by omission: | ||
| # transport::http::fetcher (SSRF), core::file_writer / core::object_store_writer | ||
| # (arbitrary/external write). The oneshot streamkit::http_input/http_output | ||
| # markers are implicitly allowed on the oneshot path and need no entry here. | ||
| allowed_nodes = [ | ||
| "audio::*", | ||
| "transport::*", | ||
| "core::*", | ||
| "video::*", | ||
| "containers::*", | ||
| "transport::moq::*", | ||
| "transport::http::mse", | ||
| "core::passthrough", | ||
| "core::file_reader", | ||
| "core::pacer", | ||
| "core::param_bridge", | ||
| "core::json_serialize", | ||
| "core::text_chunker", | ||
| "core::script", | ||
| "core::telemetry_tap", | ||
| "core::telemetry_out", | ||
| "core::sink", | ||
| "plugin::*", | ||
| ] |
There was a problem hiding this comment.
📝 Info: User role node allowlist tightened from broad wildcards to explicit list
The sample user role previously allowed transport::* and core::*, which is broader than the code's Permissions::user(). This PR replaces those with an explicit list matching the code (e.g. transport::moq::*, transport::http::mse, and an enumerated set of core:: nodes), removing transport::rtmp, core::file_writer, core::object_store_writer, etc. from the user role. This is an intentional behavior change that could break existing user-created pipelines relying on the removed nodes; worth confirming there are no shipped user//oneshot/ sample pipelines that depend on them (none currently exist under samples/pipelines/user).
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Good catch on the breakage surface. I checked the shipped pipelines: oneshot/tts_to_s3.yml, oneshot/transcode_to_s3.yml, dynamic/moq_s3_archive.yml use core::object_store_writer, and dynamic/moq_to_rtmp_composite.yml uses transport::rtmp::publish. A user can still read those samples but no longer run them.
That's intentional and not a regression relative to the secure default: the built-in Permissions::user() already denies object_store_writer (external write) and rtmp::publish (egress) — the old sample's core::*/transport::* wildcards were the outlier. Those pipelines remain runnable under admin (or a custom role that opts into the egress/write nodes). The point of this PR is to make the sample mirror that secure default.
| allowed_nodes = [ | ||
| "plugin::native::servo", # render the page (web-capture) | ||
| "video::pixel_convert", # servo RGBA -> encoder input format | ||
| "video::vp9::encoder", # encode to VP9 | ||
| "containers::webm::muxer", # mux into WebM for MSE / http_output | ||
| "transport::http::mse", # serve the live cast to the browser (MSE) | ||
| "core::pacer", | ||
| "core::sink", | ||
| ] | ||
|
|
||
| # Only the specific plugin(s) the gateway needs (example: servo for web-capture). | ||
| allowed_plugins = ["plugin::native::servo"] |
There was a problem hiding this comment.
📝 Info: Plugin enforcement ordering relied upon by gateway role
The gateway role lists plugin::native::servo in both allowed_nodes and allowed_plugins. This is required because enforcement checks is_node_allowed(kind) before the plugin allowlist (e.g. apps/skit/src/websocket_handlers.rs:55, apps/skit/src/server/sessions.rs:327). Verified the glob matching via glob::Pattern treats * ordinarily across :: (no / separators), so the literal plugin::native::servo entry and plugin::* entries resolve correctly. The new tests cover this ordering dependency.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Right — this ordering dependency (is_node_allowed runs before the plugin allowlist) was the root cause of the original gateway-role bug, where plugin::native::servo was in allowed_plugins but absent from allowed_nodes and so rejected first. The new sample_config_test assertion (gateway.is_node_allowed("plugin::native::servo")) plus the docs [!IMPORTANT] note now guard against regressing it.
The documented gateway role serves a video-only WebM cast. Page audio would also need audio::opus::encoder in allowed_nodes, since the mse node advertises codecs="vp9,opus". Noted as a future extension. Signed-off-by: streamkit-devin <devin@streamkit.dev>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The sample user role's allowed_assets only listed audio, while Permissions::user() also grants image and font assets. Add the image/font patterns so the sample matches the built-in default the comments claim to mirror, and assert the asset policy in sample_config_test. Signed-off-by: streamkit-devin <devin@streamkit.dev>
| # Least-privilege role for trusted intermediaries (e.g. the speech-gateway or | ||
| # web-capture examples) that build a small set of fixed pipelines. Avoids | ||
| # granting admin just to use HTTP-transport nodes. | ||
| create_sessions = true | ||
| destroy_sessions = true | ||
| list_sessions = true | ||
| modify_sessions = true | ||
| tune_nodes = true | ||
| load_plugins = false | ||
| delete_plugins = false | ||
| list_nodes = true | ||
| access_all_sessions = false # Only its own sessions | ||
| upload_assets = false | ||
| delete_assets = false | ||
|
|
||
| allowed_samples = [] | ||
|
|
||
| # Exactly the node kinds the web-capture / live-cast pipeline needs, nothing more. | ||
| # A plugin kind must be allowed here too: is_node_allowed() runs before the | ||
| # allowed_plugins check, so a plugin missing from allowed_nodes is rejected first. | ||
| # No fetcher (SSRF) and no file_writer (arbitrary write). The oneshot | ||
| # streamkit::http_output marker is implicitly allowed and needs no entry. | ||
| allowed_nodes = [ | ||
| "plugin::native::servo", # render the page (web-capture) | ||
| "video::pixel_convert", # servo RGBA -> encoder input format | ||
| "video::vp9::encoder", # encode to VP9 | ||
| "containers::webm::muxer", # mux into WebM for MSE / http_output | ||
| "transport::http::mse", # serve the live cast to the browser (MSE) | ||
| "core::pacer", | ||
| "core::sink", | ||
| ] | ||
|
|
||
| # Only the specific plugin(s) the gateway needs (example: servo for web-capture). | ||
| allowed_plugins = ["plugin::native::servo"] |
There was a problem hiding this comment.
📝 Info: Gateway role comment mentions speech-gateway but only enables web-capture nodes
The comment introducing the new role (samples/skit.toml:531-532) says the gateway role is for "trusted intermediaries (e.g. the speech-gateway or web-capture examples)", but the allowed_nodes/allowed_plugins list only covers the web-capture pipeline (plugin::native::servo, video::pixel_convert, video::vp9::encoder, containers::webm::muxer, transport::http::mse, core::pacer, core::sink). The speech-gateway example would need audio nodes and speech plugins (e.g. whisper/kokoro), so this role as written would not actually support it. This is a documentation/example inaccuracy rather than a functional bug, since the role is just a starter template that operators are expected to customize.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Right — the comment overclaimed. Fixed in 4f8ebd0: the comment now states the node/plugin lists are scoped to the web-capture example, and that a different gateway (e.g. speech-gateway) would swap in its own nodes/plugins (audio nodes, whisper/kokoro, etc.). The role stays a web-capture-specific starter template rather than pretending to cover both.
The comment claimed the role serves the speech-gateway too, but the node/plugin lists only cover web-capture. Clarify that the lists are web-capture-scoped and other gateways swap in their own nodes/plugins. Signed-off-by: streamkit-devin <devin@streamkit.dev>
|
|
||
| # Users cannot load plugins, so this list is empty | ||
| allowed_plugins = [] | ||
| # Users cannot load/delete plugins, but may use plugins an admin has already loaded. | ||
| allowed_plugins = ["plugin::*"] |
There was a problem hiding this comment.
📝 Info: User role permissions widened to include loaded plugins
The built-in Permissions::user() and the samples/skit.toml user role now grant plugin::* in both allowed_nodes and allowed_plugins (previously the sample TOML had allowed_plugins = []). This is a deliberate widening so users can use already-loaded plugins, but it does change the default security posture: any plugin an admin has loaded becomes usable by all user-role callers. Worth confirming this matches the intended access model, especially given native plugins run in-process without a sandbox.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Intentional — this aligns the sample with the built-in Permissions::user(), which already grants plugin::* in both lists; the sample's old allowed_plugins = [] was the outlier (and was internally inconsistent, since its allowed_nodes had core::*/transport::* but no plugin grant). The access model is unchanged from the code default: load_plugins = false still means a user cannot load a plugin — only an admin can, and users may then use what the admin has vetted. The in-process/no-sandbox risk you note is real but lives at the admin's load decision (and [plugins].allow_http_management), which is exactly where the existing IMPORTANT comment in Permissions::user() points. Operators who want a stricter posture can narrow allowed_plugins to specific kinds (as the gateway role demonstrates).
Leverage two just-merged backend PRs (auth #634, fMP4 MSE #637): - cast can now serve H.264 in fragmented MP4 via new h264-sw/h264-hw cast profiles — http::mse auto-detects fMP4 (#637), so a plain <video> plays the live stream in Safari/iOS. The cast muxer is chosen per profile now (WebM for VP9/AV1, fMP4 for H.264); VP9/WebM stays the default for crisp screen text on Chromium/Firefox. - Docs: the gateway no longer needs an admin token — the built-in user role now allows transport::http::mse (#634), so a user-role token covers the whole pipeline. (The demo's token minting is updated separately.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: streamer45 <cstcld91@gmail.com>
2 participants
Summary
userrole'sallowed_nodesexcludedtransport::http::*entirely, so HTTP-transport gateways (speech-gateway, the upcomingweb-capturefrom feat(examples): add web-capture gateway #631) were forced to use anadmintoken. Allowtransport::http::mse(serves over the caller's own request) while keepingtransport::http::fetcherdenied (arbitrary-URL fetch / SSRF) — hence a single node, not thetransport::http::*wildcard.streamkit::http_input/http_outputmarkers are not gated byallowed_nodes(the oneshot path treats them as implicitly allowed at everyis_node_allowedcall site), so they are intentionally not listed — adding them was dead config and is removed here.gatewayrole (authorization guide +samples/skit.toml) that can actually build the web-capture pipeline (servo → pixel_convert → vp9::encoder → webm::muxer → mse). Note: a plugin kind must be in bothallowed_nodesandallowed_plugins, becauseis_node_allowed()runs before the plugin check.[permissions.roles.user]withPermissions::user(): add the*_samplesflags (without whichallowed_samplesis unreachable), add the safecore::param_bridge, dropdemo/*, and replace the broadcore::*/transport::*wildcards with the explicit safe set.Review & Validation
mseallowed;fetcher,core::file_writer,core::object_store_writerdenied.userrole can no longer runtts_to_s3/transcode_to_s3/moq_s3_archive(object_store_writer) ormoq_to_rtmp_composite(rtmp::publish) — matching the built-in default; these stay runnable underadmin.cargo test -p streamkit-server permissionsand--test sample_config_test.Notes
examples/web-capturedoesn't exist yet (feat(examples): add web-capture gateway #631 in progress), so there's no example token to update; theservoplugin referenced by the gateway example is real (plugins/native/servo, used bysamples/pipelines/oneshot/web_capture.yml).Link to Devin session: https://staging.itsdev.in/sessions/88d8f7ee72f640cf8c080c08fbd46667
Requested by: @streamer45
Devin Review
4f8ebd0