Skip to content

Conversation

@rafaellehmkuhl
Copy link
Member

Cockpit.0.0.0.2025-12-10.16.12.01.mp4

Fix #1591

@ES-Alexander
Copy link
Contributor

Niiice!

Haven't tested yet, but as an initial thought, is it possible to / does it make sense to prevent custom actions and the like from using the base calls of these functions, and enforce using our managed ones?

@rafaellehmkuhl
Copy link
Member Author

Niiice!

Haven't tested yet, but as an initial thought, is it possible to / does it make sense to prevent custom actions and the like from using the base calls of these functions, and enforce using our managed ones?

We can always do a forced text replacement. Not sure if we want or not. What's your opinion on that? We would need to decide also if we want to do the replacement during writing or on save.

@ES-Alexander
Copy link
Contributor

We can always do a forced text replacement.

Given the harm users can cause to themselves/their experience, and the very-close (just safer) equivalence of our wrapped version, I think that would be fine. I guess just replacing when either function name is found without a preceding cockpit.?

I think it's ok if we consider custom functionalities like this as a slightly sandboxed version of raw JS, since if users really want to leave our guard-rails they can just modify the actual application. For everybody else it seems desirable to have some safety nets in place, especially to avoid the equivalent of forkbombing yourself or something by just clicking apply or test a few too many times 😅

We're also already allowing things like syntactic sugar for accessing data-lake variables, so I think users already accept things aren't completely normal in there... (speaking of, should this also be checked for in the compound variable definitions?)

We would need to decide also if we want to do the replacement during writing or on save.

During writing it seems annoying for the cursor to jump around unexpectedly, but it also needs to be replaced before the code can be run. I think it should be applied on save for custom actions, and on apply for DIY widgets.

Comment on lines +487 to +488
setInterval: (callback: () => void, delay?: number) => cockpitTimerManager.createManagedSetInterval(callback, delay),
setTimeout: (callback: () => void, delay?: number) => cockpitTimerManager.createManagedSetTimeout(callback, delay),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good if we can match the interface for the real ones - returning ID values that allow user code to clear them if they want to (since that could be relevant to complex functionality).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree.

@rafaellehmkuhl
Copy link
Member Author

We can always do a forced text replacement.

Given the harm users can cause to themselves/their experience, and the very-close (just safer) equivalence of our wrapped version, I think that would be fine. I guess just replacing when either function name is found without a preceding cockpit.?

I think it's ok if we consider custom functionalities like this as a slightly sandboxed version of raw JS, since if users really want to leave our guard-rails they can just modify the actual application. For everybody else it seems desirable to have some safety nets in place, especially to avoid the equivalent of forkbombing yourself or something by just clicking apply or test a few too many times 😅

We're also already allowing things like syntactic sugar for accessing data-lake variables, so I think users already accept things aren't completely normal in there... (speaking of, should this also be checked for in the compound variable definitions?)

We would need to decide also if we want to do the replacement during writing or on save.

During writing it seems annoying for the cursor to jump around unexpectedly, but it also needs to be replaced before the code can be run. I think it should be applied on save for custom actions, and on apply for DIY widgets.

Agree. Will do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JavaScript code in the JS actions and DIY widget should not accumulate on save

3 participants