Add Groovy sandbox whitelist #2057
Conversation
Summary by CodeRabbit
WalkthroughDocumentation introduces whitelist support alongside the existing blacklist for the Groovy sandbox, clarifies validation and error semantics, adds configuration examples for enabling/disabling and customizing whitelist/blacklist and Grapes across Deployer, Engine, and Studio, and updates module last-updated metadata to 4.5.0. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as Crafter Module
participant Sandbox as Groovy Sandbox
participant WL as Whitelist
participant BL as Blacklist
User->>App: Execute Groovy expression
App->>Sandbox: Evaluate(expression)
alt Sandbox enabled
opt Whitelist enabled
Sandbox->>WL: Check allowed(expression)
alt Not in whitelist
Sandbox-->>App: Error: expression not allowed
App-->>User: Validation error
else In whitelist
Sandbox->>BL: Check insecure(expression)
alt Matches blacklist
Sandbox-->>App: Error: insecure call
App-->>User: Validation error
else Not blacklisted
Sandbox-->>App: Allowed
App-->>User: Success
end
end
end
opt Whitelist disabled
Sandbox->>BL: Check insecure(expression)
alt Matches blacklist
Sandbox-->>App: Error: insecure call
App-->>User: Validation error
else Not blacklisted
Sandbox-->>App: Allowed
App-->>User: Success
end
end
opt getenvRegex configured
note over Sandbox,WL: getenvRegex permits specific getenv-style calls\n(applies even if whitelist disabled)
end
else Sandbox disabled
App-->>User: Execute without sandbox checks
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
source/includes/groovy-sandbox-configuration.rst (3)
1-2: Tighten phrasing: avoid “either … and/or …” and clarify evaluation order.Suggest clearer language and explicitly state how whitelist and blacklist interact when both are enabled.
-When a Groovy script is executed all code is validated against either a whitelist of allowed expressions and/or a blacklist -of insecure expressions, depending on what is used by your installation, to prevent code that could compromise the system. +When a Groovy script is executed, all code is validated to prevent operations that could compromise the system. Depending on your installation, validation may use a whitelist (allowed expressions), a blacklist (blocked expressions), or both. If both are enabled, an expression must be allowed by the whitelist and must not match the blacklist.
4-6: Clarify error conditions and tighten sentence structure.Split into two sentences and tie the sample error to the scenario for readability.
-When you try to execute a script that contains insecure expressions from the blacklist, or contains an expression not in -the whitelist depending on your configuration, you will see an error similar to this: +If a script contains an expression blocked by the blacklist, or (when enabled) an expression not included in the whitelist, you’ll see an error similar to the following:Also applies to: 10-10
14-16: Add explicit caution on security impact of overrides.Reinforce risk and point to the Important Notes section for best practices.
-It is recommended to keep the default configuration if possible. However, if access to one or more of the blacklisted -expressions is required, or access to one or more expressions not in the whitelist is required, it is possible to -override the blacklist and/or whitelist configuration. Configuration is global and affects all scripts on the server. +It is recommended to keep the default configuration. If you require access to blacklisted expressions or to expressions not present in the whitelist, you can override the blacklist and/or whitelist configuration. These settings are global and affect all scripts on the server. Changes may increase your attack surface—review the guidance in “Important Notes” before proceeding.source/reference/modules/engine.rst (4)
3757-3759: Fix emphasize-lines to highlight newly added whitelist properties.Currently only line 2 is emphasized; adjust to highlight the whitelist additions in this block.
-.. code-block:: properties - :linenos: - :caption: *CRAFTER_HOME/bin/apache-tomcat/shared/classes/crafter/engine/extension/server-config.properties* - :emphasize-lines: 2 +.. code-block:: properties + :linenos: + :caption: *CRAFTER_HOME/bin/apache-tomcat/shared/classes/crafter/engine/extension/server-config.properties* + :emphasize-lines: 7-10Also applies to: 3765-3768
3765-3768: Document interaction with blacklist when both are enabled.Add one line describing precedence/combination semantics to avoid ambiguity.
# Indicates if the whitelist should be enabled for all sites (this will have no effect if the sandbox is disabled) crafter.engine.groovy.sandbox.whitelist.enable=true # The location of the default whitelist to use for all sites (this will have no effect if the sandbox is disabled) crafter.engine.groovy.sandbox.whitelist.path=classpath:crafter/engine/groovy/whitelist +# Note: When both whitelist and blacklist are enabled, an expression must be present in the whitelist and not match the blacklist to be allowed.
3819-3846: Custom whitelist section reads well; add example entry and link back to properties anchor.A 1–2 line example from the default file helps readers modify correctly, and a cross-reference aids discovery.
To use a custom whitelist follow these steps: @@ -#. Update the :ref:`server-config.properties <engine-configuration-files>` configuration file to load the custom whitelist: +#. Update the :ref:`server-config.properties <engine-configuration-files>` configuration file to load the custom whitelist (see :ref:`Groovy Sandbox Properties <engine-groovy-sandbox-properties>`): @@ Now you can execute the same script without any issues. + +Example whitelist entry: + +.. code-block:: none + :caption: *Sample whitelist lines* + + # allow reading selected system properties + java.lang.System.getProperty(java.lang.String)
3850-3868: Add a security warning admonition when disabling the whitelist.Mirror the blacklist section’s caution; remind readers to scope carefully.
Disabling the Sandbox Whitelist """"""""""""""""""""""""""""""" @@ To disable the whitelist for all projects/sites update the server configuration file :ref:`server-config.properties <engine-configuration-files>`: +.. warning:: + Disabling the whitelist increases the risk of insecure scripts. Only disable in tightly controlled environments, ensure scripts don’t process untrusted input, and audit changes regularly.source/reference/modules/deployer.rst (1)
959-969: Polish getenvRegex description; clarify semantics.Minor grammar/clarity issues; suggest tightening the comments so readers understand this governs allowed args to System.getenv.
Apply this diff within the YAML code block comments:
- # List of patterns for that is allowed to call as `staticMethod java.lang.System getenv java.lang.String` parameter (regexes separated by commas) - # NOTE: This property is applied even if the whitelist is disabled + # Comma-separated regex patterns allowed as the argument to: + # staticMethod java.lang.System getenv java.lang.String + # NOTE: Applied even if the whitelist is disabled.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
source/includes/groovy-sandbox-configuration.rst(1 hunks)source/reference/modules/deployer.rst(5 hunks)source/reference/modules/engine.rst(4 hunks)source/reference/modules/studio.rst(4 hunks)
🔇 Additional comments (7)
source/reference/modules/engine.rst (1)
2-2: Version bump LGTM; verify consistency across related docs.last-updated set to 4.5.0 matches new whitelist features. Ensure Studio/Deployer pages in this PR also reflect 4.5.0 where applicable.
source/reference/modules/studio.rst (4)
2-2: Confirm version bump consistency (4.5.0).Verify this matches the actual Studio release/tag referenced elsewhere in the docs and in includes, and that all “Since 4.5.0” annotations align.
5195-5195: LGTM: external link formatting fixed.Switching to an anonymous hyperlink is fine here.
5217-5218: No action needed.Section separators only.
5267-5283: LGTM: whitelist enable toggle.The disable flag reads correctly and mirrors the blacklist section.
source/reference/modules/deployer.rst (2)
944-944: Verify highlight still correct after block edits.
:emphasize-lines: 6may no longer point to the intended YAML line after adding the whitelist block. Please confirm the highlighted line matches the property you want to call out.
1020-1050: Whitelist usage section reads well.Steps, paths, and sample override look consistent with the properties block above.
Please sanity-check that the GitHub link resolves to the correct branch/tag for 4.5.0 docs.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
source/reference/modules/deployer.rst (1)
1054-1074: Fix from earlier review is now correct.The “Disabling the Sandbox Whitelist” snippet shows enabled: false. Thanks for correcting.
🧹 Nitpick comments (4)
source/reference/modules/engine.rst (2)
3757-3768: Add precedence note between blacklist and whitelist.When both are enabled, clarify evaluation order (e.g., “whitelist denies win vs. blacklist denies win”) or link to the definitive rule in /includes/groovy-sandbox-configuration.rst.
3850-3869: Tiny grammar tweak for clarity.Use plural “expressions” and simplify phrasing.
-It is possible to disable the whitelist to allow the execution of most expressions, in -case you need to use a considerable number of the expression not included in the whitelist while keeping some basic -restrictions. +You can disable the whitelist to allow most expressions when you need many expressions that are not included in the whitelist, while still keeping basic restrictions.source/reference/modules/deployer.rst (2)
944-969: Whitelist block: tighten wording and add example for getenvRegex.
- Fix phrasing in the comment.
- Provide a concrete example pattern to guide users.
whitelist: # Indicates if the whitelist should be enabled for all targets # (this will have no effect if the sandbox is disabled) enabled: true # The location of the whitelist to use for all targets # (this will have no effect if the sandbox is disabled) path: 'file:${deployer.main.config.folderPath}/groovy/whitelist,classpath:groovy/whitelist' - # List of patterns for that is allowed to call as `staticMethod java.lang.System getenv java.lang.String` parameter (regexes separated by commas) - # NOTE: This property is applied even if the whitelist is disabled - getenvRegex: crafter_.* + # Regex patterns allowed for the argument of: `staticMethod java.lang.System getenv java.lang.String` (comma‑separated) + # NOTE: This property applies even if the whitelist is disabled + # Example: allow CRAFTER_* and AWS_* variables + getenvRegex: ^CRAFTER_.*$,^AWS_.*
980-996: Path resolution inconsistency: use the same base as other examples.You copy the blacklist to CRAFTER_HOME/bin/crafter-deployer/groovy/extension/blacklist but configure ‘file:groovy/extension/blacklist’. Use ${deployer.main.config.folderPath} for consistency and to avoid CWD ambiguity.
- sandbox: - blacklist: - # The location of the blacklist to use for all targets - # (this will have no effect if the sandbox is disabled) - path: 'file:groovy/extension/blacklist' + sandbox: + blacklist: + # The location of the blacklist to use for all targets + # (this will have no effect if the sandbox is disabled) + path: 'file:${deployer.main.config.folderPath}/groovy/extension/blacklist'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
source/includes/groovy-sandbox-configuration.rst(1 hunks)source/reference/modules/deployer.rst(5 hunks)source/reference/modules/engine.rst(4 hunks)source/reference/modules/studio.rst(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- source/includes/groovy-sandbox-configuration.rst
- source/reference/modules/studio.rst
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: alhambrav
PR: craftercms/docs#2057
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-16T14:29:45.992Z
Learning: For CrafterCMS Studio custom whitelists, users have full flexibility to add, remove, or comment expressions as needed for their installation. The whitelist system allows various operations beyond just adding expressions, including removing default expressions that may not be needed or desired for security/functionality reasons.
📚 Learning: 2025-09-16T14:29:45.992Z
Learnt from: alhambrav
PR: craftercms/docs#2057
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-16T14:29:45.992Z
Learning: For CrafterCMS Studio custom whitelists, users have full flexibility to add, remove, or comment expressions as needed for their installation. The whitelist system allows various operations beyond just adding expressions, including removing default expressions that may not be needed or desired for security/functionality reasons.
Applied to files:
source/reference/modules/engine.rstsource/reference/modules/deployer.rst
🔇 Additional comments (4)
source/reference/modules/engine.rst (2)
2-2: Version bump looks good.last-updated set to 4.5.0 aligns with new whitelist content.
3818-3846: Good section; mirrors blacklist guidance.Accurately explains copying and customizing the whitelist; restart note included.
source/reference/modules/deployer.rst (2)
2-2: Version bump OK.last-updated set to 4.5.0 matches new whitelist docs.
1020-1050: Whitelist customization section reads well.Matches Engine/Studio guidance and our prior learning that users can add/remove/comment expressions.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/reference/modules/studio.rst (1)
5200-5212: Fix classpath for custom blacklist example (points to default, not your copied file)You copy to .../classes/crafter/studio/extension/groovy/blacklist but the config references classpath:crafter/studio/groovy/blacklist. This won’t load the custom file.
Apply this diff:
- # The location of the default blacklist to use (this will have no effect if the sandbox is disabled) - studio.scripting.sandbox.blacklist.path: classpath:crafter/studio/groovy/blacklist + # The location of the custom blacklist to use (this will have no effect if the sandbox is disabled) + studio.scripting.sandbox.blacklist.path: classpath:crafter/studio/extension/groovy/blacklist
🧹 Nitpick comments (5)
source/reference/modules/engine.rst (4)
3757-3757: Emphasize all toggle properties, not just sandbox enable.
Highlight whitelist/blacklist toggles too for quick scanning.- :emphasize-lines: 2 + :emphasize-lines: 2,4,8
3765-3768: Whitelist properties: LGTM; add “Since 4.5.0” note or inline tag.
Props read well and defaults point to classpath; consider tagging these with “Since 4.5.0” like the sections below for clarity.Please also verify the default path exists in the 4.5.x engine distribution/resources.
3818-3846: Add an explicit note on combined whitelist/blacklist semantics.
Clarify how both lists interact to prevent misconfiguration.Using a Custom Whitelist ^^^^^^^^^^^^^^^^^^^^^^^^ @@ #. Update the :ref:`server-config.properties <engine-configuration-files>` configuration file to load the custom whitelist: @@ .. code-block:: properties :caption: ``CRAFTER_HOME/bin/apache-tomcat/shared/classes/crafter/engine/extension/server-config.properties`` @@ crafter.engine.groovy.sandbox.whitelist.path=classpath:crafter/engine/extension/groovy/whitelist + +.. note:: + When both whitelist and blacklist are enabled, an expression must be allowed by the whitelist and not denied by the blacklist. + Disabling the whitelist does not disable the blacklist; blacklist restrictions still apply if enabled.
3850-3868: Call out security impact when disabling whitelist.
Add a brief admonition to make the risk and remaining protections explicit.Disabling the Sandbox Whitelist ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ It is possible to disable the whitelist to allow the execution of most expressions, in case you need to use a considerable number of the expression not included in the whitelist while keeping some basic restrictions. + +.. warning:: + Disabling the whitelist broadens what scripts can execute. Ensure the blacklist remains enabled and appropriately strict. @@ # Indicates if the whitelist should be enabled for all sites (this will have no effect if the sandbox is disabled) crafter.engine.groovy.sandbox.whitelist.enable=falsesource/reference/modules/studio.rst (1)
5178-5188: Add whitelist keys to the “Groovy Sandbox Properties” blockFor completeness and discoverability, include whitelist.enable/path alongside blacklist.*.
Apply this diff:
# Indicates if the blacklist should be enabled (this will have no effect if the sandbox is disabled) studio.scripting.sandbox.blacklist.enable: true # The location of the default blacklist to use (this will have no effect if the sandbox is disabled) studio.scripting.sandbox.blacklist.path: classpath:crafter/studio/groovy/blacklist + # Indicates if the whitelist should be enabled (this will have no effect if the sandbox is disabled) + studio.scripting.sandbox.whitelist.enable: true + # The location of the default whitelist to use (this will have no effect if the sandbox is disabled) + studio.scripting.sandbox.whitelist.path: classpath:crafter/studio/groovy/whitelist
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
source/reference/modules/engine.rst(4 hunks)source/reference/modules/studio.rst(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: alhambrav
PR: craftercms/docs#2057
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-16T14:29:46.013Z
Learning: For CrafterCMS Studio custom whitelists, users have full flexibility to add, remove, or comment expressions as needed for their installation. The whitelist system allows various operations beyond just adding expressions, including removing default expressions that may not be needed or desired for security/functionality reasons.
📚 Learning: 2025-09-16T14:29:46.013Z
Learnt from: alhambrav
PR: craftercms/docs#2057
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-16T14:29:46.013Z
Learning: For CrafterCMS Studio custom whitelists, users have full flexibility to add, remove, or comment expressions as needed for their installation. The whitelist system allows various operations beyond just adding expressions, including removing default expressions that may not be needed or desired for security/functionality reasons.
Applied to files:
source/reference/modules/studio.rstsource/reference/modules/engine.rst
🔇 Additional comments (6)
source/reference/modules/engine.rst (2)
2-2: Version bump looks correct; verify cross‑doc consistency.
Confirm 4.5.0 is the first release with whitelist support and that “last‑updated” is updated in related Deployer/Studio docs in this PR.
3800-3801: No actionable change.
Formatting spacer only.source/reference/modules/studio.rst (4)
2-2: Update metadata looks correct:last-updated set to 4.5.0 is appropriate for the new sections.
5194-5196: Blacklist section link tweak LGTMHyperlink formatting fix is good.
5266-5282: Whitelist disabling section reads wellClear, consistent with blacklist section.
5297-5308: Grapes download property addition LGTMDefaults and guidance are clear.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
source/reference/modules/deployer.rst (2)
944-969: Whitelist block: clarity nit and grammar
- Good addition (enabled, path, getenvRegex). Minor grammar tweak in the getenvRegex comment.
Apply this small copy edit:
- # List of patterns for that is allowed to call as `staticMethod java.lang.System getenv java.lang.String` parameter (regexes separated by commas) + # List of patterns allowed as the parameter for `staticMethod java.lang.System getenv java.lang.String` (regexes separated by commas) # NOTE: This property is applied even if the whitelist is disabled getenvRegex: crafter_.*
980-995: Align custom blacklist path with earlier patternUse the same folderPath placeholder used elsewhere to avoid confusion with relative paths.
- sandbox: - blacklist: - # The location of the blacklist to use for all targets - # (this will have no effect if the sandbox is disabled) - path: 'file:groovy/extension/blacklist' + sandbox: + blacklist: + # The location of the blacklist to use for all targets + # (this will have no effect if the sandbox is disabled) + path: 'file:${deployer.main.config.folderPath}/groovy/extension/blacklist'source/reference/modules/studio.rst (2)
5182-5188: Document whitelist properties alongside blacklist.For completeness and quick discovery, add whitelist properties to the “Groovy Sandbox Properties” block (mirrors the blacklist entries).
Apply this diff:
# Indicates if the sandbox should be enabled studio.scripting.sandbox.enable: true # Indicates if the blacklist should be enabled (this will have no effect if the sandbox is disabled) studio.scripting.sandbox.blacklist.enable: true # The location of the default blacklist to use (this will have no effect if the sandbox is disabled) studio.scripting.sandbox.blacklist.path: classpath:crafter/studio/groovy/blacklist + # Indicates if the whitelist should be enabled (this will have no effect if the sandbox is disabled) + studio.scripting.sandbox.whitelist.enable: true + # The location of the default whitelist to use (this will have no effect if the sandbox is disabled) + studio.scripting.sandbox.whitelist.path: classpath:crafter/studio/groovy/whitelist
5195-5213: Call it “custom blacklist” in the override example.The YAML sample points to your custom file; label it accordingly to avoid confusion.
Apply this diff:
- # The location of the default blacklist to use (this will have no effect if the sandbox is disabled) + # The location of the custom blacklist to use (this will have no effect if the sandbox is disabled) studio.scripting.sandbox.blacklist.path: classpath:crafter/studio/extension/groovy/blacklist
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
source/reference/modules/deployer.rst(5 hunks)source/reference/modules/engine.rst(4 hunks)source/reference/modules/studio.rst(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: alhambrav
PR: craftercms/docs#2057
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-16T14:29:46.013Z
Learning: For CrafterCMS Studio custom whitelists, users have full flexibility to add, remove, or comment expressions as needed for their installation. The whitelist system allows various operations beyond just adding expressions, including removing default expressions that may not be needed or desired for security/functionality reasons.
📚 Learning: 2025-09-16T14:29:46.013Z
Learnt from: alhambrav
PR: craftercms/docs#2057
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-16T14:29:46.013Z
Learning: For CrafterCMS Studio custom whitelists, users have full flexibility to add, remove, or comment expressions as needed for their installation. The whitelist system allows various operations beyond just adding expressions, including removing default expressions that may not be needed or desired for security/functionality reasons.
Applied to files:
source/reference/modules/studio.rst
📚 Learning: 2025-09-02T20:40:32.613Z
Learnt from: alhambrav
PR: craftercms/docs#2042
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-02T20:40:32.613Z
Learning: When reviewing CrafterCMS Studio documentation, property names in configuration examples should match the actual Studio source code implementation, even if the underlying Git config keys are different. Studio may have a translation layer that maps Studio property names to actual Git configuration keys.
Applied to files:
source/reference/modules/studio.rst
🔇 Additional comments (11)
source/reference/modules/engine.rst (4)
2-2: Metadata bump to 4.5.0 looks correctNo issues.
3765-3768: Whitelist properties added — LGTMThe properties are named consistently and default to enabled. Good addition alongside the blacklist.
Please confirm Engine 4.5.0 actually exposes these exact keys:
- crafter.engine.groovy.sandbox.whitelist.enable
- crafter.engine.groovy.sandbox.whitelist.path
3818-3846: “Using a Custom Whitelist” section reads wellInstructions and paths are clear and consistent with blacklist guidance.
3850-3868: “Disabling the Sandbox Whitelist” — sample is correctExample shows enabled=false as expected. Good.
source/reference/modules/deployer.rst (2)
1020-1046: “Using a Custom Whitelist” — LGTMClear steps, consistent defaults, and restart note included.
1060-1074: “Disabling the Sandbox Whitelist” — sample is correctMatches section intent with enabled: false.
This was previously flagged and has been addressed; noting it’s correct now.
source/reference/modules/studio.rst (5)
2-2: Version bump looks good.
[last-updated] updated to 4.5.0 appropriately.
5234-5244: Whitelist section intro reads well.Nice addition; link and version tag are clear. The flexibility to add/remove/comment matches prior guidance.
5278-5282: Whitelist disable example is clear.
Defaults and semantics read correctly.
5299-5305: Grapes download toggle looks good.
Clear default and how to enable.
5254-5259: Fix classpath and label in custom whitelist example (path order is wrong).The copy path is .../classes/crafter/studio/extension/groovy/whitelist, but the config uses classpath:crafter/studio/groovy/extension/whitelist. This will break loading.
Apply this diff:
- # The location of the default whitelist to use (this will have no effect if the sandbox is disabled) - studio.scripting.sandbox.whitelist.path: classpath:crafter/studio/groovy/extension/whitelist + # The location of the custom whitelist to use (this will have no effect if the sandbox is disabled) + studio.scripting.sandbox.whitelist.path: classpath:crafter/studio/extension/groovy/whitelist
source/reference/modules/engine.rst
Outdated
| .. code-block:: yaml | ||
| :caption: *CRAFTER_HOME/bin/apache-tomcat/shared/classes/crafter/engine/extension/server-config.properties* | ||
| :emphasize-lines: 3 | ||
| # Indicates if @Grab annotations should download grapes | ||
| # If false, only already downloaded grapes will be available | ||
| crafter.engine.groovy.grapes.download.enabled=false | ||
| Automatic grapes download is disabled by default. Set ``crafter.engine.scripting.grapes.download.enabled`` to true to | ||
| enable automatic grapes download. |
There was a problem hiding this comment.
Fix code block language and property name inconsistency in Grapes section
- The code block caption points to server-config.properties, but the block is marked as YAML. Use properties.
- The narrative below references crafter.engine.scripting.grapes.download.enabled, while the example shows crafter.engine.groovy.grapes.download.enabled. Keep a single, correct key.
Apply this diff to fix both issues:
-.. code-block:: yaml
+.. code-block:: properties
:caption: *CRAFTER_HOME/bin/apache-tomcat/shared/classes/crafter/engine/extension/server-config.properties*
:emphasize-lines: 3
# Indicates if @Grab annotations should download grapes
# If false, only already downloaded grapes will be available
crafter.engine.groovy.grapes.download.enabled=falseAnd adjust the enabling sentence:
-Automatic grapes download is disabled by default. Set ``crafter.engine.scripting.grapes.download.enabled`` to true to
- enable automatic grapes download.
+Automatic grapes download is disabled by default. Set ``crafter.engine.groovy.grapes.download.enabled`` to true to enable automatic grapes download.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .. code-block:: yaml | |
| :caption: *CRAFTER_HOME/bin/apache-tomcat/shared/classes/crafter/engine/extension/server-config.properties* | |
| :emphasize-lines: 3 | |
| # Indicates if @Grab annotations should download grapes | |
| # If false, only already downloaded grapes will be available | |
| crafter.engine.groovy.grapes.download.enabled=false | |
| Automatic grapes download is disabled by default. Set ``crafter.engine.scripting.grapes.download.enabled`` to true to | |
| enable automatic grapes download. | |
| .. code-block:: properties | |
| :caption: *CRAFTER_HOME/bin/apache-tomcat/shared/classes/crafter/engine/extension/server-config.properties* | |
| :emphasize-lines: 3 | |
| # Indicates if @Grab annotations should download grapes | |
| # If false, only already downloaded grapes will be available | |
| crafter.engine.groovy.grapes.download.enabled=false | |
| Automatic grapes download is disabled by default. Set ``crafter.engine.groovy.grapes.download.enabled`` to true to enable automatic grapes download. |
🤖 Prompt for AI Agents
In source/reference/modules/engine.rst around lines 3887 to 3896, the code block
is incorrectly marked as YAML and uses the wrong property name; change the
code-block language to "properties", update the shown property key to
"crafter.engine.scripting.grapes.download.enabled=false" (keeping
emphasize-lines: 3 as intended), and update the explanatory sentence below to
say "Set ``crafter.engine.scripting.grapes.download.enabled`` to true to enable
automatic grapes download."
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
source/reference/modules/engine.rst (1)
3754-3769: Whitelist properties added — consider documenting precedence with blacklistThe new whitelist enable/path properties look good. Please add one sentence clarifying how whitelist and blacklist interact when both are enabled (evaluation order, tie-breaking). This avoids ambiguity for operators.
If precedence is already covered in an included doc (e.g., groovy-sandbox-important-notes), please point to it from here.
source/reference/modules/studio.rst (2)
5189-5191: Whitelist properties added correctly.
Properties and defaults look consistent with existing blacklist settings. Optional: add a short “Since 4.5.0” note here for quick scannability.
5214-5216: Clarify comment: this example is for a custom blacklist.
Change “default” → “custom” to avoid confusion in the “Using a Custom Blacklist” section.Apply this diff:
- # The location of the default blacklist to use (this will have no effect if the sandbox is disabled) + # The location of the custom blacklist to use (this will have no effect if the sandbox is disabled) studio.scripting.sandbox.blacklist.path: classpath:crafter/studio/extension/groovy/blacklistsource/reference/modules/deployer.rst (3)
944-969: Tighten getenvRegex comment for clarityImprove grammar and make the API reference explicit.
Apply this diff:
- :emphasize-lines: 6 + :emphasize-lines: 6 @@ - whitelist: + whitelist: @@ - # List of patterns for that is allowed to call as `staticMethod java.lang.System getenv java.lang.String` parameter (regexes separated by commas) - # NOTE: This property is applied even if the whitelist is disabled + # Comma-separated regex patterns allowed for the argument to System.getenv(String) when invoked via Groovy staticMethod + # NOTE: This property applies even if the whitelist is disabled getenvRegex: crafter_.*
1020-1050: “Using a Custom Whitelist”: align wording with example path and minor copy edits
- “classpath” doesn’t match the example path under the config directory. Suggest saying “Deployer config directory.”
- Minor readability tweak on Step 2.
Apply this diff:
-#. Copy the default whitelist file to your classpath, for example: +#. Copy the default whitelist file to the Deployer config directory, for example: @@ -#. Add, remove or comment (adding a ``#`` at the beginning of the line) the expressions that your scripts require +#. Add, remove, or comment out (add a ``#`` at the beginning of the line) the expressions that your scripts require
1054-1074: “Disabling the Sandbox Whitelist”: copy edit and confirm corrected example
- Grammar improvement in the paragraph.
- YAML example correctly sets enabled: false (fix verified).
Apply this diff:
-It is possible to disable the whitelist to allow the execution of most expressions, in -case you need to use a considerable number of the expression not included in the whitelist while keeping some basic -restrictions. +You can disable the whitelist to allow most expressions when you need to use a considerable number of expressions not included in the whitelist, while still keeping basic restrictions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
source/reference/modules/deployer.rst(5 hunks)source/reference/modules/engine.rst(4 hunks)source/reference/modules/studio.rst(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: alhambrav
PR: craftercms/docs#2057
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-16T14:29:46.013Z
Learning: For CrafterCMS Studio custom whitelists, users have full flexibility to add, remove, or comment expressions as needed for their installation. The whitelist system allows various operations beyond just adding expressions, including removing default expressions that may not be needed or desired for security/functionality reasons.
📚 Learning: 2025-09-16T14:29:46.013Z
Learnt from: alhambrav
PR: craftercms/docs#2057
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-16T14:29:46.013Z
Learning: For CrafterCMS Studio custom whitelists, users have full flexibility to add, remove, or comment expressions as needed for their installation. The whitelist system allows various operations beyond just adding expressions, including removing default expressions that may not be needed or desired for security/functionality reasons.
Applied to files:
source/reference/modules/studio.rst
📚 Learning: 2025-09-02T20:40:32.613Z
Learnt from: alhambrav
PR: craftercms/docs#2042
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-02T20:40:32.613Z
Learning: When reviewing CrafterCMS Studio documentation, property names in configuration examples should match the actual Studio source code implementation, even if the underlying Git config keys are different. Studio may have a translation layer that maps Studio property names to actual Git configuration keys.
Applied to files:
source/reference/modules/studio.rst
🔇 Additional comments (14)
source/reference/modules/engine.rst (4)
2-2: Version bump looks correct:last-updated reflects 4.5.0 as intended.
3818-3846: Custom whitelist section reads wellClear steps (copy, edit, point path, restart). Nice alignment with Studio guidance about adding/removing/commenting expressions (consistent with user preferences).
3850-3869: Whitelist disabling section looks goodProperty name and scope are consistent with the properties defined above.
3895-3897: Fix property key in narrative to match example (groovy vs scripting)The sentence references crafter.engine.scripting.grapes.download.enabled, but the example (and surrounding keys) use crafter.engine.groovy.grapes.download.enabled. Keep a single, correct key.
Apply this diff:
-Automatic grapes download is disabled by default. Set ``crafter.engine.scripting.grapes.download.enabled`` to true to - enable automatic grapes download. +Automatic grapes download is disabled by default. Set ``crafter.engine.groovy.grapes.download.enabled`` to true to enable automatic grapes download.To double-check which key is authoritative across the docs, run:
#!/bin/bash # Search for both property variants across the repo to ensure consistency rg -n -C2 -P 'crafter\.engine\.(groovy|scripting)\.grapes\.download\.enabled' --type rst --type mdsource/reference/modules/studio.rst (7)
2-2: Version bump looks good.
[last-updated] updated to 4.5.0 is consistent with new whitelist/grapes features.
5199-5199: Good: default blacklist link.
Link points to the correct resources path/branch.
5251-5254: Classpath copy target matches guidance.
Copy path under shared/classes looks correct for classpath loading.
5255-5255: Instruction phrasing aligns with intended flexibility.
Allowing add/remove/comment is accurate for custom whitelists (per your clarification).
5284-5285: Whitelist disable flag looks correct.
Property name and context align with the blacklist equivalent.
5307-5310: Grapes download toggle is correct.
Default disabled with clear guidance; good addition.
5259-5263: Fix incorrect classpath for custom whitelist example.
The path is reversed; it should be crafter/studio/extension/groovy/whitelist to match the copy step above. Also change “default” → “custom”.Apply this diff:
- # The location of the default whitelist to use (this will have no effect if the sandbox is disabled) - studio.scripting.sandbox.whitelist.path: classpath:crafter/studio/groovy/extension/whitelist + # The location of the custom whitelist to use (this will have no effect if the sandbox is disabled) + studio.scripting.sandbox.whitelist.path: classpath:crafter/studio/extension/groovy/whitelistsource/reference/modules/deployer.rst (3)
2-2: Metadata bump to 4.5.0 looks correctNo issues.
1000-1001: No action neededSection separator lines are fine.
1081-1105: Grapes Download section reads well and matches property path
- Duplicate “:caption:” issue is gone.
- Property path in the narrative matches the YAML (deployer.main.scripting.grapes.download.enabled).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
source/reference/modules/engine.rst (2)
757-768: Document both whitelist and blacklist configuration properties consistently.The new whitelist properties are correctly added. However, to improve documentation clarity, consider explicitly stating that the whitelist and blacklist work together as complementary security mechanisms when both are enabled.
Consider adding a brief explanatory comment above the whitelist properties:
# The location of the default blacklist to use for all sites (this will have no effect if the sandbox is disabled) crafter.engine.groovy.sandbox.blacklist.path=classpath:crafter/engine/groovy/blacklist +# Whitelist configuration (works in conjunction with blacklist when both are enabled) # Indicates if the whitelist should be enabled for all sites (this will have no effect if the sandbox is disabled) crafter.engine.groovy.sandbox.whitelist.enable=true
850-868: Clarify the relationship between whitelist and blacklist when both are enabled.Based on the retrieved learning, users have full flexibility with custom whitelists. Consider adding a brief note explaining how whitelist and blacklist interact when both are enabled to help users understand the security model.
Add a note after the whitelist disabling section:
# Indicates if the whitelist should be enabled for all sites (this will have no effect if the sandbox is disabled) crafter.engine.groovy.sandbox.whitelist.enable=false + +.. note:: + When both whitelist and blacklist are enabled, they work together as complementary security mechanisms. + The whitelist defines what is explicitly allowed, while the blacklist defines what is explicitly denied.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
source/reference/modules/engine.rst(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: alhambrav
PR: craftercms/docs#2057
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-16T14:29:46.013Z
Learning: For CrafterCMS Studio custom whitelists, users have full flexibility to add, remove, or comment expressions as needed for their installation. The whitelist system allows various operations beyond just adding expressions, including removing default expressions that may not be needed or desired for security/functionality reasons.
🔇 Additional comments (3)
source/reference/modules/engine.rst (3)
2-2: LGTM!The version update from 4.3.1 to 4.5.0 is consistent with the addition of whitelist support functionality described in the AI summary.
818-870: LGTM!The new whitelist configuration section follows the same pattern as the existing blacklist section, maintaining consistency in documentation structure and providing clear step-by-step instructions.
878-897: Address inconsistency identified in past review comment.There's an existing code block language and property name inconsistency issue that needs to be resolved before this section is complete.
The code block should use
propertiesinstead ofyaml, and there's a property name mismatch that needs to be corrected as detailed in the previous review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
source/includes/groovy-grape-install.rst (1)
2-7: Clarify grape root and platform variant
- Consider noting that -Dgrape.root must match the runtime grape root used by Deployer/Engine/Studio so preinstalled grapes are picked up (e.g., ${deployer.main.config.folderPath} if applicable).
- Add the Windows invocation (groovy/bin/grape.bat) for completeness.
Apply this small copy edit near the command:
To install, go to the ``CRAFTER_HOME/bin`` directory, then run the following command: .. code-block:: bash :caption: *Install new grapes from the command line* - ./groovy/bin/grape -Dgrape.root=. install <GROUP ID> <ARTIFACT ID> <VERSION> + ./groovy/bin/grape -Dgrape.root=. install <GROUP ID> <ARTIFACT ID> <VERSION> + +.. note:: + Ensure the value passed to ``-Dgrape.root`` is the same location your runtime uses to resolve grapes. + On Windows, use: ``groovy\\bin\\grape.bat -Dgrape.root=. install <GROUP ID> <ARTIFACT ID> <VERSION>``.source/reference/modules/deployer.rst (3)
959-969: Tighten wording for getenvRegex (and clarify multiple patterns)
- Grammar: “List of patterns that are allowed …” (remove “for”).
- Clarify delimiter for multiple patterns and that the value is parsed as a comma‑separated list.
- # List of patterns for that is allowed to call as `staticMethod java.lang.System getenv java.lang.String` parameter (regexes separated by commas) + # List of regex patterns that are allowed as the parameter to: + # staticMethod java.lang.System getenv java.lang.String + # Multiple patterns can be provided, separated by commas.
1060-1074: Minor grammar fix (“expressions”)-It is possible to disable the whitelist to allow the execution of most expressions, in -case you need to use a considerable number of the expression not included in the whitelist while keeping some basic +It is possible to disable the whitelist to allow the execution of most expressions, in +case you need to use a considerable number of expressions not included in the whitelist while keeping some basic restrictions.
1112-1115: Good inclusion of CLI install doc; add cross‑ref hintOptionally add a short sentence above the include to tie together pre‑download vs. auto‑download scenarios.
-""""""""""""""""""""""""""""""""""""""" -Installing Grapes from the Command Line -""""""""""""""""""""""""""""""""""""""" +""""""""""""""""""""""""""""""""""""""" +Installing Grapes from the Command Line +""""""""""""""""""""""""""""""""""""""" +Use this when automatic grapes download is disabled or when you prefer to pre‑install dependencies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
source/includes/groovy-grape-install.rst(1 hunks)source/reference/modules/deployer.rst(5 hunks)source/reference/modules/engine.rst(4 hunks)source/reference/modules/studio.rst(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: alhambrav
PR: craftercms/docs#2057
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-16T14:29:46.013Z
Learning: For CrafterCMS Studio custom whitelists, users have full flexibility to add, remove, or comment expressions as needed for their installation. The whitelist system allows various operations beyond just adding expressions, including removing default expressions that may not be needed or desired for security/functionality reasons.
📚 Learning: 2025-09-16T14:29:46.013Z
Learnt from: alhambrav
PR: craftercms/docs#2057
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-16T14:29:46.013Z
Learning: For CrafterCMS Studio custom whitelists, users have full flexibility to add, remove, or comment expressions as needed for their installation. The whitelist system allows various operations beyond just adding expressions, including removing default expressions that may not be needed or desired for security/functionality reasons.
Applied to files:
source/reference/modules/studio.rst
📚 Learning: 2025-09-02T20:40:32.613Z
Learnt from: alhambrav
PR: craftercms/docs#2042
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-02T20:40:32.613Z
Learning: When reviewing CrafterCMS Studio documentation, property names in configuration examples should match the actual Studio source code implementation, even if the underlying Git config keys are different. Studio may have a translation layer that maps Studio property names to actual Git configuration keys.
Applied to files:
source/reference/modules/studio.rst
🔇 Additional comments (2)
source/reference/modules/deployer.rst (2)
1020-1052: Whitelist section reads well; matches flexibility to add/remove/comment entriesThis aligns with prior guidance that users can add/remove/comment expressions as needed. LGTM.
Please confirm the example path and property reference resolve at runtime on Deployer:
- Copy path: CRAFTER_HOME/bin/crafter-deployer/config/groovy/whitelist
- YAML path: file:${deployer.main.config.folderPath}/groovy/whitelist,classpath:groovy/whitelist
If confirmed, consider mirroring the same “file:${deployer.main.config.folderPath}…” pattern in the “Custom Blacklist” section for consistency.
1085-1106: Grapes download defaults and path look good; keep property path consistentLGTM that automatic download is disabled by default and the narrative references deployer.main.scripting.grapes.download.enabled, matching the YAML path.
Double‑check that the default in the application is indeed “false” so docs and runtime align.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
source/reference/modules/deployer.rst (2)
941-969: Consider emphasizing whitelist.enabled as wellThe code block only emphasizes one line. To help readers, also emphasize the whitelist enable toggle alongside the sandbox and blacklist toggles. No behavior change—purely a readability win.
1031-1036: Clarify “classpath” wording for whitelist file locationYou say “copy … to your classpath” but the example path is an override in the config folder read via a file: URL. Recommend clarifying to “copy to the config folder (or another location you reference via file: or classpath:)” to avoid confusion. Content stays the same.
source/reference/modules/studio.rst (6)
5188-5191: Clarify whitelist/blacklist evaluation order.Great to see whitelist support documented. Please add one sentence noting how whitelist and blacklist interact when both are enabled (e.g., evaluation order and which list “wins” on conflicts) to avoid operator confusion.
5214-5216: Comment says “default” but path points to custom blacklist.Change the comment to avoid implying it’s the built-in list.
- # The location of the default blacklist to use (this will have no effect if the sandbox is disabled) + # The location of the custom blacklist to use (this will have no effect if the sandbox is disabled) studio.scripting.sandbox.blacklist.path: classpath:crafter/studio/extension/groovy/blacklist
5255-5256: Whitelist step wording can mislead.Readers may read “remove … expressions that your scripts require” as disallowing needed expressions. Suggest clarifying allow vs. deny explicitly:
-#. Add, remove or comment (adding a ``#`` at the beginning of the line) the expressions that your scripts require +#. Add or uncomment the expressions you wish to allow; remove or comment out any you wish to disallow, as needed for your installationBased on learnings
5261-5263: Comment says “default” but path points to custom whitelist.Match the wording to the custom path:
- # The location of the default whitelist to use (this will have no effect if the sandbox is disabled) + # The location of the custom whitelist to use (this will have no effect if the sandbox is disabled) studio.scripting.sandbox.whitelist.path: classpath:crafter/studio/extension/groovy/whitelist
5277-5286: Call out effect when disabling whitelist but keeping blacklist enabled.Add a short note that disabling the whitelist falls back to blacklist-only enforcement (if enabled), and include a reminder that this broadens allowed surface area.
5299-5314: Nice addition; add quick operational tip.Consider a one-line note: in air‑gapped environments keep this disabled and preinstall grapes (next section), otherwise enabling may require outbound egress and proxy settings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
source/reference/modules/deployer.rst(5 hunks)source/reference/modules/studio.rst(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: alhambrav
PR: craftercms/docs#2057
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-16T14:29:46.013Z
Learning: For CrafterCMS Studio custom whitelists, users have full flexibility to add, remove, or comment expressions as needed for their installation. The whitelist system allows various operations beyond just adding expressions, including removing default expressions that may not be needed or desired for security/functionality reasons.
📚 Learning: 2025-09-16T14:29:46.013Z
Learnt from: alhambrav
PR: craftercms/docs#2057
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-16T14:29:46.013Z
Learning: For CrafterCMS Studio custom whitelists, users have full flexibility to add, remove, or comment expressions as needed for their installation. The whitelist system allows various operations beyond just adding expressions, including removing default expressions that may not be needed or desired for security/functionality reasons.
Applied to files:
source/reference/modules/studio.rst
📚 Learning: 2025-09-02T20:40:32.613Z
Learnt from: alhambrav
PR: craftercms/docs#2042
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-02T20:40:32.613Z
Learning: When reviewing CrafterCMS Studio documentation, property names in configuration examples should match the actual Studio source code implementation, even if the underlying Git config keys are different. Studio may have a translation layer that maps Studio property names to actual Git configuration keys.
Applied to files:
source/reference/modules/studio.rst
🔇 Additional comments (2)
source/reference/modules/deployer.rst (1)
1069-1074: Whitelist disable sample now correct (enabled: false)Thanks for fixing the contradiction; the snippet now actually disables the whitelist and matches the heading.
source/reference/modules/studio.rst (1)
2-2: Version metadata update looks good.
Ticket reference or full description of what's in the PR
Add Groovy sandbox whitelist https://github.com/craftersoftware/craftercms/issues/1156