Fix permissions to ensure tracker can only send events#64
Fix permissions to ensure tracker can only send events#64
Conversation
mnutt
commented
Jan 28, 2026
mnutt
commented
- Separate permissions for tracking script vs manager
- Inspect headers to ensure permissions are valid
|
I tried to make a fix at one point before we decided that the update would be too hard to be worth it. IIRC I had to unpack your package because the npm packages were no longer available or something. Curious how you're running this? |
|
Oh I see you've updated everything first. |
| var template = "<script>" + $("#tracking_template").html() + "</scr" + "ipt>\n"; | ||
| template = template.replace('$API_PROTO', document.location.protocol); | ||
| window.addEventListener("message", messageListener); | ||
| window.parent.postMessage({renderTemplate: {rpcId: "0", template: template}}, "*"); |
There was a problem hiding this comment.
I keep forgetting what forSharing actually does, but it's not what it sounds like. IIRC it makes the share show up in your "Share access" instead of your "Webkeys". For an API token I'd guess you want "Webkeys" but that's up to you.
Beyond that I don't think it affects permissions much. But I guess doublecheck my claims here.
@ocdtrekkie Is there anything else?
There was a problem hiding this comment.
forSharing : Boolean (optional) true if this token should represent the anonymous user. You can use this to detach the token from the user who created it
So I think we want it to be true here?
There was a problem hiding this comment.
Ah, yeah that was the other thing. But iirc you show up as anonymous when used as a share link but not as an API key, or maybe the other way around, I forget. In any case, goofy behavior. You can try it out.
There was a problem hiding this comment.
Yeah forSharing "detaches it from you", so IMHO it should be true for any key you intend to make public or give to someone else. I don't think it has a major security implication but it does move it from webkeys (which is sort of your API keys) to sharing (which is who you shared it with).
I think it should be true here.
|
|
||
| var io = new Server(server, ioOptions); | ||
|
|
||
| io.use(function(socket, next) { |
There was a problem hiding this comment.
So when I tried to do this I put this in a different place, but you know your codebase better. Here's what I'd make sure of:
- Dashboard requires view permission (you cover that in this clause)
- Tracker requires tracker permission (I don't see that in this PR, and I noticed it was absent previously)
- Websockets, where the actual data flows, require view permission (I'm not sure if this clause covers it)
There was a problem hiding this comment.
Yes, this code here protects the web sockets with view-data permissions, and the above in defaultHandler protects regular http. I should add a guard on the tracker for completeness.
|
Yes, it had bitrotted and I had to do a bunch of updates to even get things running. (I believe the node.js it installed was no longer available from nodesource) |
3 participants
