-
Notifications
You must be signed in to change notification settings - Fork 228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ability to inject JavaScript #354
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting aside, for the moment, the impending retreading of the conversation around security concerns of injecting arbitrary JavaScript that we already had in #137...
Please add the locale file changes for the strings that this adds.
Please fix the clang-format error.
@RytoEX FYI, the ubuntu workflow is using clang12, I used clang13 - I took the liberty to downgrade to clang12 for this, but please make sure to let us know if clang12 is fixed right now - thanks. |
We require using clang-format 12.0.0 at the moment because that's what our CI workflows use. The formatted output produced by clang-format changes between any given version. We plan to update the clang-format version requirement, hopefully within the next few months. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer if this setting's boolean state was logged. I don't think we need to log the full JavaScript string, but I would want to know that a user enabled it on a source and if the JavaScript string is empty or not.
How would you like to log it, just as a string somewhere "The user activated javascript in the browser source %s" ? |
Any chance this makes it into OBS 28? 🤔 Ideally with injecting in |
I'd love to, but at this point I gave up. I know a couple of users simply compiled this themselves, but aside from that I guess the main way of communicating would be to go to one of the chatting platforms (i guess discord) and poke relevant people. |
Any chance there's a pre-built version with the injection PR or cheat-sheet somewhere for how-to somewhere? Or maybe even an add-on version with an alternate browser with the feature? I see in the README that I can't build the browser as a standalone and it has to be part of the OBS studio build. I'm not at all a C++ dev and have been banging my head on the wall trying to figure out how to build with this feature but haven't gotten anywhere... I was super excited when I searched around for this and found someone had already solved it... Then super sad to find that it's apparently just been stonewalled for what appears to be like 4 years?? I can totally understand wanting to have plenty of warnings for users that they can expose themselves if they don't know what they're doing with custom JS... But why not at least have the option when the work's already been done? |
Well, I did talk to the people over at Discord regarding this topic quite a while ago and actually was asked to hold my horses until they updated to a newer CEF (Chrome Embedded Framework) version as the sandbox changed quite a bit. |
Wow that would be fantastic!
I'm using windows.
Thanks!!
…________________________________
From: KuhnChris ***@***.***>
Sent: Friday, September 30, 2022, 4:39 AM
To: obsproject/obs-browser ***@***.***>
Cc: scriptpup ***@***.***>; Comment ***@***.***>
Subject: Re: [obsproject/obs-browser] #137 Try #2 - Injecting Javascript (PR #354)
Well, I did talk to the people over at Discord regarding this topic quite a while ago and actually was asked to hold my horses until they updated to a newer CEF (Chrome Embedded Framework) version as the sandbox changed quite a bit.
As there still isn't any update on this, I might aswell try to build it again. For what OS do you need this? Windows, Linux, MacOS-X?
—
Reply to this email directly, view it on GitHub<#354 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABPAZPU36RAPG66PP2P3PL3WA2RLLANCNFSM5PKN66JA>.
You are receiving this because you commented.Message ID: ***@***.***>
|
Usually I loathe to distribute binaries, as one can never be sure that they are safe, but since it was requested a couple of times: To recreate this, simply follow the entire build process outlined here: https://obsproject.com/wiki/build-instructions-for-windows Let me know if that helps you! |
Just as a note, we definitely haven't forgotten about this and the request that users have for it. We have made steps towards properly supporting this safely with the recent CEF update, but keep in mind that it is our opinion that injecting arbitrary JS is very risky, and we're not 100% sold on supporting it at this point, unless it can be done very safely. We haven't had time to get around to experimenting with the CEF sanboxing feature, which I feel comfortable saying is going to be bare minimum requirement for this to be merged and released by us. |
Doesn't injected JS just run in the same JS context as the page it's injected in, and just the same as any normal browser source page in general? What's the difference between a user injecting JS from somewhere on the internet vs setting a browser source url from somewhere on the internet, security wise? And I get the concern about that but even every actual browser I'm aware of lets users open a dev console and do whatever, and the general workarounds I've seen for people needing to inject JS into OBS web sources were either using And I don't mean to be rude here sorry if I come off like that, I get you're all very busy and have way more pressing issues and features on the docket than JS injection into browser sources, but it's just the seemingly large concern about the security of this specific feature I don't really get in the context of everything else in OBS. |
Thank you very very much @kuhnchris! The binaries work perfectly. I understand your point about it being better to build from source... This is definitely an area I'm lacking in and need to learn how to properly build C++ from source myself. That being said, for the current problem I was facing the plugins you posted work perfectly! 🙏 @Fenrirthviti I'm glad to hear it's not dead! This would be a great out-of-the-box feature, and I also totally understand not wanting to release something that the owning team isn't 100% confident in (especially if it's related to security). I don't think I've ever gotten replies from maintainers or even thread creators this quickly before - This community is clearly fantastic! Thank you again internet strangers for helping me solve my problem, although you owed me absolutely nothing :) |
Thanks for that info, I haven't really been following around. Thanks again and all the best, |
There is no open PR, and we haven't begun internal testing. Enabling it isn't the issue, it's testing all the use cases of browser source currently to ensure there are no unintended side effects that is the blocker. |
Pardon my naivity, but, if there is such a focus on use cases, why isn't there a unit-test/test framework in place that covers them? Is there any way we could help collect the required use-cases? If so, we could technically help with creating those for you guys, especially if "implementing isn't the issue". I haven't seen any roadmap or anything regarding this, so the community is kind of in the air, but it could be me not looking in the right place to begin with, so I'll excuse myself before-hand. |
The answer to all of that is the same: Nobody has spent time on it. The reasons for that are varied and numerous. As with most open-source projects, people tend to work on things that interest them. With only a single full time developer on OBS as of the writing of this message, we need to be careful with how we assign that resource, and we have no direct control over what anyone else on the project wants to work on. Every new feature, every pull request, and every suggestion is the most important thing to the person who wants it. We have to weight and prioritize based on things that help as much of the majority of the userbase as we can, which is not easy. If you, or anyone else, are offering to put that work in to help push this forward, I'm sure it would be welcomed. |
Most definitly, since I'm not the only "active" contribute over here, it would make sense to channel these efforts. |
Things are getting cluttered here, and I wouldn't want any reviewer having to read through a bunch of comments about project governance or priorities. If there are any questions about project governance or priorities, please reach out to us on our Discord. Myself (RytoEX#5545) or @Fenrirthviti (Len#9999) can answer any questions about project priorities, how things get decided, or what the best path forward is for tackling a specific item. Moving forward, let's keep discussion focused on the actual code and concerns that we've previously raised. On a high level, something that is required for us to even consider merging this is making sure that we can enable the CEF/Chromium sandbox on all platforms (Windows, macOS, Linux/*nix) without breaking any functionality (service integration, browser docks/panels, browser sources, alerts, local files, locally served web pages, etc.). You, or anyone else, are welcome to investigate and try to tackle those tasks in a separate PR, or in multiple PRs if necessary for scope/testing. Do not try to tack on the sandbox to this PR. Note, that this is not a guarantee that we will simply merge any PR that appears and claims to solve those problems. We would need to see a demonstration that the PR both addresses our requirement (enabling the sandbox) and does not break anything (show that you've done the testing, ideally by providing test files/cases that we can verify). Now that we've touched on project governance and priorities and our prerequisites for this specific item... To answer the specifics of why we delayed looking into this, there were major architectural changes that we wanted to address first before revisiting CEF sandboxing (see: obsproject/obs-studio#4560, obsproject/obs-studio#5155, and #290) as well as updating CEF (first from CEF 75/3770 to 95/4638; then from 95/4638 to 103/5060; see #323). Hopefully, that answers the bulk of the questions of "what is this waiting on" from our side of things. On the specifics of this PR, you would need to rebase the changes on to the current master branch of this repository, for starters. As-is, this is not mergeable. Finally, please keep comments on this PR on-topic to the specifics of this PR and its code and implementation requirements/details. If this PR devolves into off-topic discussion, it will be locked. |
Thanks for clarifying - let's get back on topic with this PR then, separate discussions somewhere else, I agree. Ad clarifying the sandboxing/CEF upgrade issue: OK, thanks, that makes a lot more sense now. Ad this PR: Wait, it isn't mergable? I did merge current master into javascript yesterday when building it, but maybe I did something wrong? Again, it wasn't my intend to off-topic the discussion here - so let's keep it at the |
Hello ! I know you guys are super busy but do you think this will be merged into next OBS version ? 😄 |
I assume not, but I will get to rebasing the changes to master hopefully this weekend. |
you didn't rebase properly; as a result there are 43 (!!!) commits ... Please correct this, thank you. |
Squashed commit of the following: commit eb52a6c Merge: bbce3fc 49fb735 Author: KuhnChris <kuhnchris@kuhnchris.eu> Date: Fri Apr 14 14:26:30 2023 +0200 Merge branch 'obsproject:master' into javascript commit bbce3fc Merge: 3e3803b a736208 Author: KuhnChris <kuhnchris+github@kuhnchris.eu> Date: Fri Sep 30 20:22:30 2022 +0100 Merge branch 'javascript' of https://github.com/kuhnchris/obs-browser into javascript commit 3e3803b Author: Kuhn Chris <kuhnchris+github@kuhnchris.eu> Date: Fri Feb 25 17:48:02 2022 +0100 Added info-log, simplified injection, sorting commit e814265 Author: Kuhn Chris <kuhnchris+github@kuhnchris.eu> Date: Fri Feb 25 16:40:54 2022 +0100 clang13 v.s. clang12 differences. commit a4493c1 Author: Kuhn Chris <kuhnchris+github@kuhnchris.eu> Date: Fri Feb 25 16:31:57 2022 +0100 Locales; CLang Fixes commit 3a57f41 Author: Kuhn Chris <kuhnchris+github@kuhnchris.eu> Date: Fri Feb 25 15:05:47 2022 +0100 Update to latest commit a736208 Author: Kuhn Chris <kuhnchris+github@kuhnchris.eu> Date: Fri Feb 25 17:48:02 2022 +0100 Added info-log, simplified injection, sorting commit 7368db8 Author: Kuhn Chris <kuhnchris+github@kuhnchris.eu> Date: Fri Feb 25 16:40:54 2022 +0100 clang13 v.s. clang12 differences. commit cb78d99 Author: Kuhn Chris <kuhnchris+github@kuhnchris.eu> Date: Fri Feb 25 16:31:57 2022 +0100 Locales; CLang Fixes commit 61b2a31 Author: Kuhn Chris <kuhnchris+github@kuhnchris.eu> Date: Fri Feb 25 15:05:47 2022 +0100 Update to latest Code Checks (CI-checkformat.sh)
Here we go, single commit, squashed. Feel free to try to compile it. Edit, also @pkviet no idea where you got 43 commits, but I assume you may have based it from the first commit, but it should be a single one commit now. |
See above the comments from @RytoEX |
oh that's what you meant, well, I squashed them, so it should be fine, you're welcome. |
@Warchamp7 so we should close/discard this PR then too, since you closed #87 ? |
Hello, if this possible to use this obs obs 27.2.4 ? this is working fine on 28 but i use 27 please anyone share link for obs 27, Thanks |
Closing this per comments in #87 |
Would it be possible to get this as an OBS extension then? I and a lot of people really need this functionality and using the Chrome Debugger mode is outright silly and impractical for doing daily streams having to manage browsers to tweak things every time. With more and more apps denying us the ability to make our own things such as Discord Stream Kit still having the Discord RPC API locked away from the public and their terrible HTML design making it impossible for CSS overrides to do the job any more, it's becoming impossible to do anything with Browser Sources as they are. If it's a separate extension we can install into OBS then would that satisfy the "You are really sure you what to do this" requirement? Please, I really don't want to have to go to Stream Labs for this one missing feature. |
@Dav-Edward Do you have an example use-case? It is my belief that there are far too many security concerns with arbitrary (and potentially silent) javascript injection to outweigh the potential uses. There are certainly ways that a plugin could achieve this behaviour, but I do not see us supporting it out of the box.
I'm not quite sure I understand how Streamlabs affects this? Streamlabs Desktop is an OBS fork and has the exact same limitations for Browser Sources. |
I thought Stream Lab's version did have the ability to inject Javascript into their Browser Sources based on comments in one of the duplicate pull requests. As for use case, absolutely I do. Discord Stream Kit ( https://discord.com/streamkit ) used to be resonably moddable using CSS for people to do things like rearrange positions of user avatars, set custom avatars for the users, and various other features until two things happened. First they optimized the website using some code optimizer which stripped away all ID tags on objects in the page making specific user references impossible so you can't override specific users with specific avatars or positions, and they enabled Strict-XSS which means it's now impossible to do any image overrides as the web page won't let you reference images outside of the Discord domain. Discord more than 4 years ago released https://discord.com/developers/docs/topics/rpc but then ripped it away again shortly thereafter from the public meaning there is absolutely no way to make your own version of the Discord Stream Kit by writing your own website as you cannot get access to the Discord RPC APIs. Only one known developer, https://reactive.fugi.tech ever got access to the Discord RPC APIs by grandfathering in and no one else has been able to make an alternative to Discord Stream Kit other than FugiTech. Those two are litterally the only two choices available for any app of any kind to monitor voice activity in Discord voice chat for the purposes of OBS overlays. All kinds of Twitch\YouTube\etc. VTuber streamers rely on Discord Streamkit to show voice activity of users in their Discord during streams. But now that Stream Kit is so locked down and limited, people cannot set custom avatars for users appearing in their Discord which poses problems as streamers can't manage what images people might set as their Discord avatar which can cause a TOS violation on the streaming platform of a user enters the voice chat with an explicit avatar. Twitch streamers like myself heavily used the CSS override feature to make it so users could have custom avatars that appear during streams, and strangers would be given a safe, default avatar to represent themselves when they enter the voice chat. It alow allowed for all kinds of creativity for example being able to reposition avatars so that users avatars 'fit together' to make up a scene when in voice chat. The only way we can get Discord Streamkit to be even remotly useable these days is by starting OBS into Remote Debugging mode and firing up Chome or Firefox to then inject Javascript overrides from the web browser which is a very tedious process every single time we start our streams having to run the overrides on every relevant browser source in OBS. Attached is an example of what used to be possible to accomplish with CSS but now requires direct intervention with Javascript to accomplish. If security is the concern, this being made a separate plugin would be perfectly acceptable as a solution. The attack surface would be minimal as only users specifically meaning to use the feature would go out of their way to install the plugin or turn on the functionality. I and many, many, streamers I know need this functionality as right now FugiTech is our absolute last option now and it's very limited in what modifications you can do as it much like Discord Stream Kit does not use ID references in HTML tags so you can't do anything useful with CSS. FugiTech uses Canvas and Blob objects making CSS useless to attempt to modify or reposition avatars. |
I think the real solution to this particular case is petitioning Fugi to add more customizability or Discord to amend their classes to be more friendly to CSS modifications rather than JS injection. That is a particularly overkill solution as I noted previously. That being said, modifying the Discord Stream Kit via CSS only is still perfectly doable. Example CSS:
In the above case I've hidden peoples normal avatars and created a fake default one using your GitHub profile icon. I've set an override for myself specifically to use my own icon, and made it hue-shifted when I'm talking resulting in the below appearance Notes: I've censored the peoples names in Photoshop after the fact, the the example CSS links to Imgur as GitHub does not allow hotlinking |
@Warchamp7 There is absolutely no way we can strongarm Discord to fix their website, there has been hundreds of upvotes on the Discord feedback website for them to change the Stream Kit website back and they have not, and asking FugiTech to add a ton of additional features to their website is not sensible. I'm both impressed and confused how you managed to do what you did. Ever since Discord changed their Stream Kit website to remove the reference IDs we completely lost the ability to hide Flexboxes, Move/Reposition Flexboxes, and override avatars properly. Before the change we used to be able to write code like this to control the position, animation, and content of the flexboxes:
Then, after their first set of changes, we completely lost the ability to target the flexboxes and had to resort to only changing the images this way:
One part that has me very confused is in your example, were you actually able to reference images OUTSIDE of Discord?? They have Strict-XSS enabled. When we tried to reference external resorces before, the external resources got shot down. How did you do that? I used to have the script refer to a repo I managed with a good old Is it actually plausible to still be able to: That would be preferable than us having to fire up Chrome Remote Debugging for every single streamer and cracking open the Developer console every time. Javascript access does give us the ability to also potentially hook into states the website has but doesn't use like 'User Self-Muted' and 'User Self-Deafended' states which the website oddly doesn't actually use despite the API calls being in there. |
@Warchamp7 also, if there is a way to resolve this. I will gladly spread the word, as in all sorts of forums, Discords, and Reddit VTubers are streaming into the void beating their heads against the wall trying to get back the ability to override avatars and flexbox locations/visbility. I'll let everyone know far and wide your solution with credit if this is actually possible. The only way I knew of to target the flexboxes is as follows:
However, OBS does not support the :has statement. I asked about it here a while back: https://www.reddit.com/r/obs/comments/zxr3j1/has_css_support_for_more_advanced_custom_css/ |
Yes my example is tested and working in both Chrome and OBS itself. It's possible they reverted that restriction. If it was re-enabled again, another potential workaround would be to upload the icons to Discord and use the attachment URL. I doubt they would block a subset of their own CDN. A cumbersome workaround, sure, but again doable without JS injection.
As far as I can tell there are no flex box elements in the current version of Discord's Stream Kit, but realistically you could hide any element with an appropriate CSS selector, yes.
As above, there's no flexboxes (Is this simply your understanding of a DOM element?) but yes you could reposition things
Based off the above example yes. Plus the potential workaround.
I'm not sure why that would be needed unless you are frequently updating this CSS and constantly having to update it on the browser source. In that case it would be possible to write a Lua script or obs-websocket script that could fetch the CSS and update the Browser Sources properties with the new Custom CSS.
The has() selector was added to Chromium in version 105. OBS is currently on version 103 of Chromium. This will be updated to a newer version some day, but I cannot say which version that will be or when it will happen (It will not be in OBS 30). I'd have to know exactly what you're trying to accomplish in order to propose an alternative. Feel free to hop in the OBS Discord and ping me (@Warchamp7) in #plugins-and-tools or another channel, as I'd like to avoid cluttering up this PR with development work ;D |
Joke's on you, they recently added expiration tokens to upload links. So this solution is only good for 3 to 12 months. https://www.reddit.com/r/discordapp/comments/16uy0an/not_sure_if_this_is_real/ |
Again, it's a worst case scenario solution. It currently does not appear to be needed anyway. |
Description
This is the 2nd attempt of PR #137 - ability to inject javascript into a websource
Motivation and Context
#87
How Has This Been Tested?
Tested locally, also some other users who require the functionality tested the change.
Types of changes
Checklist: