[cloud] Provide ability to disable executing modified pxl scripts #2062
Conversation
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
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
…cation Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
ddelnano
force-pushed
the
ddelnano/enforce-scripts-arent-modified
branch
from
cda686a to
af0447c
Compare
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
ddelnano
force-pushed
the
ddelnano/enforce-scripts-arent-modified
branch
from
094bbc7 to
4270147
Compare
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
aimichelle
reviewed
Dec 17, 2024
Member
aimichelle
left a comment
aimichelle
left a comment
There was a problem hiding this comment.
For a slightly better experience, should we also update the UI so that the editor doesn't allow the user to make any modifications? That way they don't have to wait to edit the script, then try to run the script, in order to know it's not going to work.
ddelnano
force-pushed
the
ddelnano/enforce-scripts-arent-modified
branch
2 times, most recently
from
0a31fa1 to
13ffb77
Compare
…ing and renamed to match other SCRIPT_ prefixed settings Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
ddelnano
force-pushed
the
ddelnano/enforce-scripts-arent-modified
branch
from
13ffb77 to
1c57303
Compare
NickLanam
approved these changes
Dec 23, 2024
Member
NickLanam
left a comment
NickLanam
left a comment
There was a problem hiding this comment.
UI adjustments LGTM, with the comment below.
| scrollBeyondLastLine: false, | ||
| fontFamily: COMMON_THEME.typography.monospace.fontFamily, | ||
| readOnly: this.props.isReadOnly === true, | ||
| readOnly: this.props.isReadOnly === true || SCRIPT_MODIFICATION_DISABLED, |
Member
There was a problem hiding this comment.
Notably, CodeEditor doesn't show the user why it's read-only, and assumes the caller will tell them why (I don't remember if any existing caller does so, though - they might not).
Having readOnly set directly inside of CodeEditor this way breaks that assumption further, showing nothing and giving no indication that it should. Might be worth consideration in a future change!
Member
Author
There was a problem hiding this comment.
I've changed this in favor of having each instance of the component to provide a read only reason. See testing done screenshots for more details.
ddelnano
commented
Jan 7, 2025
| env PL_KRATOS_SERVICE; | ||
| env SCRIPT_BUNDLE_URLS; | ||
| env SCRIPT_BUNDE_DEV; | ||
| env SCRIPT_BUNDLE_DEV; |
Member
Author
There was a problem hiding this comment.
This is a typo but something that's only used for development.
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
…tend and proxy service Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
ddelnano
force-pushed
the
ddelnano/enforce-scripts-arent-modified
branch
from
49e06d1 to
b0098ca
Compare
Member
Author
|
@aimichelle please see the latest testing done screenshots. The UI now enables the read only setting for the |
aimichelle
approved these changes
Jan 10, 2025
ddelnano
added a commit
to ddelnano/pixie
that referenced
this pull request
Aug 6, 2025
…xie-io#2062) Summary: [cloud] Provide ability to disable executing modified pxl scripts Certain security conscious users are hesitant to use Pixie because without RBAC anyone with Pixie UI access can write arbitrary BPF code (bpftrace integration), access or export arbitrary data (modifying pxl scripts, writing export scripts). This change aims to address this concern with a global setting to prevent the ability to execute modified scripts. When an adhoc script is executed, the cloud will hash the contents of the script and check it against the scripts known to the scriptmgr service. If it is contained in the scriptmgr service, the script will be allowed to execute. Note: this does not prevent users from writing new export scripts. Since the query broker can source its scripts from a configmap as of pixie-io#1326, this is deemed as an appropriate mitigation for cluster admins and I'll follow up with UI support to reflect that a vizier is in "configmap mode". Relevant Issues: N/A Type of change: /kind feature Test Plan: The following checks were performed - [x] New tests verify the scriptmgr and api service changes work - [x] Skaffold'ed to a testing cluster and verified script modification is blocked and unmodified scripts are allowed to run. In addition to this, the code editor in the UI is made read only and shows an explanation <details><summary>Screenshots</summary>    </details> Changelog Message: Pixie Cloud can now disable executing modified pxl scripts via the `PL_SCRIPT_MODIFICATION_DISABLED` key in the `pl-script-bundle-config` ConfigMap. See reference manifests for more details. --------- Signed-off-by: Dom Del Nano <ddelnano@gmail.com> GitOrigin-RevId: d12b805
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary: [cloud] Provide ability to disable executing modified pxl scripts
Certain security conscious users are hesitant to use Pixie because without RBAC anyone with Pixie UI access can write arbitrary BPF code (bpftrace integration), access or export arbitrary data (modifying pxl scripts, writing export scripts). This change aims to address this concern with a global setting to prevent the ability to execute modified scripts. When an adhoc script is executed, the cloud will hash the contents of the script and check it against the scripts known to the scriptmgr service. If it is contained in the scriptmgr service, the script will be allowed to execute.
Note: this does not prevent users from writing new export scripts. Since the query broker can source its scripts from a configmap as of #1326, this is deemed as an appropriate mitigation for cluster admins and I'll follow up with UI support to reflect that a vizier is in "configmap mode".
Relevant Issues: N/A
Type of change: /kind feature
Test Plan: The following checks were performed
Screenshots
Changelog Message: Pixie Cloud can now disable executing modified pxl scripts via the
PL_SCRIPT_MODIFICATION_DISABLEDkey in thepl-script-bundle-configConfigMap. See reference manifests for more details.