Update docs and tests according to PR feedback

Author: Cole-Greer

docs/src/reference/gremlin-applications.asciidoc

@@ -1339,26 +1339,17 @@ Gremlin Server supports three mechanisms to configure authorization:
 
 . With the `traversalSources` YAML configuration, one can declare `TraversalSource` instances with
 link:https://tinkerpop.apache.org/docs/x.y.z/reference/#traversalstrategy[TraversalStrategy instances] applied via
-a Gremlin query, some of which can serve for authorization purposes (`ReadOnlyStrategy`,
+a Gremlin expression, some of which can serve for authorization purposes (`ReadOnlyStrategy`,
 `LambdaRestrictionStrategy`, `VertexProgramRestrictionStrategy`, `SubgraphStrategy`, `PartitionStrategy`,
 `EdgeLabelVerificationStrategy`), provided that users are not allowed to remove or modify these `TraversalStrategy`
 instances afterwards. For example:
-+
+
 [source,yaml]
 ----
 traversalSources: {
-  g: {graph: graph, query: "g.withStrategies(ReadOnlyStrategy)"}}
-----
-+
-Alternatively, the deprecated `ScriptFileGremlinPlugin` approach can still be used with a Groovy script:
-+
-[source,yaml]
-----
-scriptEngines: {
-  gremlin-groovy: {
-    plugins: {
-      org.apache.tinkerpop.gremlin.jsr223.ScriptFileGremlinPlugin: {files: [scripts/empty-sample.groovy]}}}}
+  g: {graph: graph, gremlinExpression: "g.withStrategies(ReadOnlyStrategy)"}}
 ----
+
 . Administrators can configure an authorizer class, an implementation of the `Authorizer` interface. An authorizer receives
 a request before it is executed and it can decide to pass or deny the request, based on the information it has available
 on the requesting user or can seek externally.
@@ -1654,12 +1645,14 @@ groupsandbox: [usersandbox, marko]
 
 
 [[script-execution]]
-==== Protecting Script Execution
+==== Protecting Groovy Script Execution
 
-NOTE: As of 4.0.0, Groovy is no longer required for server initialization.  The `scriptEngines` configuration
-described in this section is only needed when Groovy script execution is explicitly enabled for remote script
-submission.  See <<server-lifecycle-hooks>> and <<server-traversal-sources>> for the preferred YAML-based
-initialization approach.
+NOTE: TinkerPop currently provides two `GremlinScriptEngine` implementations, the grammar-based `GremlinLangScriptEngine`,
+and the Groovy-based `GremlinGroovyScriptEngine`. `GremlinLangScriptEngine` is the default implementation and
+recommended for most usages. This engine evaluates scripts through our gremlin grammar and parser, and thus isn't prone
+to the same arbitrary script evaluation risks as the `GremlinGroovyScriptEngine`. The following section details the
+process of hardening the `GremlinGroovyScriptEngine` if necessary. The simplest and safest recommendation is to avoid
+enabling `GremlinGroovyScriptEngine` in applications where hardening is warranted.
 
 It is important to remember that Gremlin Server exposes `GremlinScriptEngine` instances that allows for remote execution
 of arbitrary code on the server.  Obviously, this situation can represent a security risk or, more minimally, provide
@@ -2059,14 +2052,7 @@ included. The relevant configuration from the Gremlin Server YAML looks like thi
 [source,yaml]
 ----
 traversalSources: {
-  g: {graph: graph, query: "g.withStrategies(ReferenceElementStrategy)"}}
-----
-
-Alternatively, using the deprecated Groovy init script approach:
-
-[source,groovy]
-----
-globals << [g : traversal().with(graph).withStrategies(ReferenceElementStrategy)]
+  g: {graph: graph, gremlinExpression: "g.withStrategies(ReferenceElementStrategy)"}}
 ----
 
 This configuration is global to Gremlin Server and therefore all methods of connection will always return elements

docs/src/upgrade/release-4.x.x.asciidoc

@@ -34,28 +34,38 @@ complete list of all the modifications that are part of this release.
 
 ==== Gremlin Server Initialization Without Groovy
 
-Gremlin Server no longer requires Groovy for server initialization. Three new YAML configuration mechanisms replace
-the Groovy init scripts that were previously used to create `TraversalSource` bindings, load data, and run lifecycle
-hooks.
+Previous versions of Gremlin Server relied on the Groovy script engine for basic server initialization; binding
+traversal sources, loading data, and running lifecycle hooks all required Groovy init scripts. This meant the Groovy
+script engine was active by default, even for users who only needed to send standard Gremlin queries. An active
+scripting engine capable of executing arbitrary code introduces a security surface that is difficult to harden and
+easy to misconfigure.
 
-===== Auto-Created TraversalSources
+As of this release, Groovy is completely disabled by default throughout the entire server lifecycle. Server
+initialization is handled through new declarative YAML configuration, and standard Gremlin queries are processed by the
+`gremlin-lang` engine.  Users who explicitly need Groovy script execution can still opt in, but it is no longer
+required for any standard usage.
 
-Any graph defined in the `graphs` section that does not have an explicit `traversalSources` entry will automatically
-get a `TraversalSource` binding. A graph named `graph` is bound to `g`; all others are bound to `g_<name>`.
+===== Simplified Server Configuration
 
-A minimal server configuration is now:
+The most basic server setup, initializing a basic graph with a traversal source, previously required a Groovy init
+script just to create the `g` binding.  Now, any graph defined in the `graphs` section automatically gets a
+`TraversalSource` according to these rules:
+
+* A graph named `graph` is implicitly bound to `g`
+* All others are bound to `g_<graph-name>` (e.g. `modern` gets `g_modern`)
+
+A fully functional minimal configuration is now simply:
 
 [source,yaml]
 ----
 graphs: {
   graph: conf/tinkergraph-empty.properties}
 ----
 
-This automatically creates a `g` binding — no init script needed.
-
-===== Declarative `traversalSources`
+===== Strategy Configuration via `traversalSources`
 
-A new `traversalSources` YAML section allows explicit `TraversalSource` creation with optional strategy configuration:
+For cases that require custom traversalSource naming or strategies on a `TraversalSource` (e.g. `ReadOnlyStrategy`), the
+`traversalSources` YAML section provides explicit control without scripting:
 
 [source,yaml]
 ----
@@ -66,14 +76,18 @@ traversalSources: {
 
 Each entry specifies:
 
-- `graph` (required) — references a key in the `graphs` section
-- `gremlinExpression` (optional) — a Gremlin expression evaluated with a base traversal source bound as `g`
-- `language` (optional) — which script engine to use for the expression (defaults to `gremlin-lang`, or the sole
+- `graph` (required): references a key in the `graphs` section
+- `gremlinExpression` (optional): a Gremlin expression evaluated with a base traversal source bound as `g`
+- `language` (optional): which script engine to use for the expression (defaults to `gremlin-lang`, or the sole
   configured engine if only one is present)
 
-===== Java-Based `lifecycleHooks`
+Graphs with explicit `traversalSources` entries are excluded from the implicitly defined traversal sources described in
+the previous section.
 
-A new `lifecycleHooks` YAML section replaces Groovy-based `LifeCycleHook` creation:
+===== Java-Based Lifecycle Hooks
+
+For startup and shutdown logic that goes beyond declarative configuration such as loading sample data, initializing
+caches, or custom setup, Java-based `LifeCycleHook` implementations can now replace Groovy init scripts:
 
 [source,yaml]
 ----
@@ -83,16 +97,14 @@ lifecycleHooks:
 ----
 
 Each entry specifies a `className` implementing `LifeCycleHook` and an optional `config` map passed to the hook's
-`init()` method. The built-in `TinkerFactoryDataLoader` supports datasets: `modern`, `classic`, `crew`, `grateful`,
-`sink`, and `airroutes`.
+`init()` method. The built-in `TinkerFactoryDataLoader` supports datasets: `airroutes`, `modern`, `classic`, `crew`,
+`grateful`, and `sink`.
 
-===== Deprecation of Groovy Init Scripts
+===== Migrating from Groovy Init Scripts
 
 Creating `TraversalSource` and `LifeCycleHook` instances via Groovy init scripts is now deprecated. Existing scripts
-continue to work, but a deprecation warning is logged at startup. Migrate to the YAML-based configuration described
-above.
-
-===== Migration Examples
+continue to work when `GremlinGroovyScriptEngine` is explicitly configured, but a deprecation warning is logged at
+startup. Support for Groovy script initialization and customization may be dropped in a future release.
 
 *Before (Groovy init script):*
 
@@ -117,7 +129,7 @@ lifecycleHooks:
   - { className: org.apache.tinkerpop.gremlin.server.util.TinkerFactoryDataLoader, config: {graph: graph, dataset: modern}}
 ----
 
-The `g` binding is auto-created from the `graph` entry. No `scriptEngines` section is needed.
+The `g` binding is implicitly created from the `graph` entry. No `scriptEngines` section is needed.
 
 == TinkerPop 4.0.0-beta.2
 

gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java

@@ -177,6 +177,9 @@ public Settings overrideSettings(final Settings settings) {
             case "shouldTimeOutRemoteTraversal":
                 settings.evaluationTimeout = 500;
                 break;
+            case "ensureScriptEngineDefaultsToGremlinLang":
+                settings.scriptEngines = new HashMap<>();
+                break;
             case "shouldBlowTheWorkQueueSize":
                 settings.gremlinPool = 1;
                 settings.maxWorkQueueSize = 1;
@@ -1088,4 +1091,54 @@ public void shouldParseFloatLiteralWithoutSuffixDependingOnLanguage() {
         assertEquals(new BigDecimal("1.0"), client.submit("g.inject(1.0)", gremlinGroovy).one().getObject());
         assertEquals(new BigDecimal("-123.456"), client.submit("g.inject(-123.456)", gremlinGroovy).one().getObject());
     }
+
+    @Test
+    public void ensureScriptEngineDefaultsToGremlinLang() {
+        final Cluster cluster = TestClientFactory.open();
+        final Client client = cluster.connect();
+
+        try {
+            // should handle traversal with no explicit language
+            final RequestOptions withAlias = RequestOptions.build().addG("gmodern").create();
+            assertEquals(6L, client.submit("g.V().count()", withAlias).one().getLong());
+
+            // should handle script with explicit gremlin-lang
+            final RequestOptions langOptions = RequestOptions.build().language("gremlin-lang").addG("gmodern").create();
+            assertEquals(6L, client.submit("g.V().count()", langOptions).one().getLong());
+
+            // should reject non-Gremlin expressions
+            try {
+                client.submit("2+2", langOptions).all().get();
+                fail("Should have failed for non-Gremlin expression");
+            } catch (Exception ex) {
+                final ResponseException re = (ResponseException) ex.getCause();
+                assertEquals(HttpResponseStatus.BAD_REQUEST, re.getResponseStatusCode());
+                assertEquals("MalformedQueryException", re.getRemoteException());
+            }
+
+            // in gremlin-groovy '1g' is a valid BigDecimal but in gremlin-lang it should be '1m'
+            try {
+                client.submit("g.inject(1g)", langOptions).all().get();
+                fail("Should have failed for Groovy-specific syntax");
+            } catch (Exception ex) {
+                final ResponseException re = (ResponseException) ex.getCause();
+                assertEquals(HttpResponseStatus.BAD_REQUEST, re.getResponseStatusCode());
+                assertEquals("MalformedQueryException", re.getRemoteException());
+            }
+
+            // gremlin-lang BigDecimal syntax should work
+            assertEquals(BigDecimal.ONE, client.submit("g.inject(1m)", langOptions).one().getObject());
+
+            // explicit gremlin-groovy should fail when engine is not configured
+            try {
+                client.submit("2+2", groovyRequestOptions).all().get();
+                fail("Should have failed for unavailable gremlin-groovy engine");
+            } catch (Exception ex) {
+                final ResponseException re = (ResponseException) ex.getCause();
+                assertEquals(HttpResponseStatus.BAD_REQUEST, re.getResponseStatusCode());
+            }
+        } finally {
+            cluster.close();
+        }
+    }
 }

gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/SettingsTest.java

@@ -22,6 +22,8 @@
 import org.yaml.snakeyaml.Yaml;
 import org.yaml.snakeyaml.constructor.Constructor;
 
+import java.io.File;
+import java.io.FileInputStream;
 import java.io.InputStream;
 
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -33,6 +35,11 @@
 
 public class SettingsTest {
 
+    private static InputStream getMinimalConfigStream() throws Exception {
+        final File confDir = new File(System.getProperty("build.dir"), "../conf");
+        return new FileInputStream(new File(confDir, "gremlin-server-min.yaml"));
+    }
+
     private static class CustomSettings extends Settings {
         public String customValue = "localhost";
 
@@ -64,7 +71,7 @@ public void defaultCustomValuesAreHandledCorrectly() throws Exception {
 
     @Test
     public void scriptEnginesDefaultsToGremlinLangWhenAbsentFromYaml() throws Exception {
-        final InputStream stream = SettingsTest.class.getResourceAsStream("gremlin-server-minimal.yaml");
+        final InputStream stream = getMinimalConfigStream();
         final Settings settings = Settings.read(stream);
 
         assertThat(settings.scriptEngines, is(notNullValue()));
@@ -103,7 +110,7 @@ public void traversalSourcesParsedFromYaml() throws Exception {
 
     @Test
     public void traversalSourcesDefaultsToEmptyMapWhenAbsentFromYaml() throws Exception {
-        final InputStream stream = SettingsTest.class.getResourceAsStream("gremlin-server-minimal.yaml");
+        final InputStream stream = getMinimalConfigStream();
         final Settings settings = Settings.read(stream);
 
         assertThat(settings.traversalSources, is(notNullValue()));
@@ -128,7 +135,7 @@ public void lifecycleHooksParsedFromYaml() throws Exception {
 
     @Test
     public void lifecycleHooksDefaultsToEmptyListWhenAbsentFromYaml() throws Exception {
-        final InputStream stream = SettingsTest.class.getResourceAsStream("gremlin-server-minimal.yaml");
+        final InputStream stream = getMinimalConfigStream();
         final Settings settings = Settings.read(stream);
 
         assertThat(settings.lifecycleHooks, is(notNullValue()));