fix: harden default request timeout, GraphQL error leakage, and LSP exit halt

serve-application/src/main/java/build/serve/application/ServerApplication.java

@@ -26,11 +26,14 @@
 import build.serve.foundation.error.DefaultErrorHandler;
 import build.serve.foundation.error.ErrorHandler;
 import build.serve.foundation.option.ListenAddress;
+import build.serve.foundation.option.MaxRequestSize;
+import build.serve.foundation.option.RequestTimeout;
 import build.serve.foundation.option.ShutdownTimeout;
 import build.serve.foundation.option.TlsConfig;
 import build.serve.foundation.routing.Router;
 import build.serve.security.SecurityHeadersMiddleware;
 import build.serve.transport.http.HttpTransport;
+import build.serve.transport.http.TlsOptions;
 import build.spawn.application.Addressable;
 import build.spawn.application.Lifecycle;
 
@@ -164,6 +167,17 @@ void recorder(final TelemetryRecorder recorder) {
             this.recorder = Objects.requireNonNull(recorder, "recorder must not be null");
         }
 
+        /**
+         * Obtains the {@link TelemetryRecorder} bound to this server, for use by
+         * {@link #configure()} when wiring components (e.g. {@code GraphQlSchema}) that need
+         * somewhere to report their own errors instead of defaulting to stdout/stderr.
+         *
+         * @return the {@link TelemetryRecorder}
+         */
+        protected TelemetryRecorder recorder() {
+            return recorder;
+        }
+
         /**
          * Configures and returns the {@link Router} for this server.
          * <p>
@@ -231,7 +245,8 @@ public void start(final ListenAddress listenAddress,
 
             final var address = new InetSocketAddress(listenAddress.value(), listenPort.get());
 
-            this.transport = new HttpTransport(address, 0, applySecurityHeaders(router), errorHandler(), recorder);
+            this.transport = new HttpTransport(address, 0, applySecurityHeaders(router), errorHandler(),
+                MaxRequestSize.DEFAULT, RequestTimeout.DEFAULT, recorder);
             this.transport.start();
 
             this.shutdownHook = new Thread(this::stop, "serve-shutdown");
@@ -275,7 +290,7 @@ public void start(final ListenAddress listenAddress,
             final var address = new InetSocketAddress(listenAddress.value(), listenPort.get());
 
             this.transport = HttpTransport.https(address, 0, applySecurityHeaders(router), errorHandler(),
-                tls.sslContext(), recorder);
+                MaxRequestSize.DEFAULT, tls.sslContext(), TlsOptions.defaults(), RequestTimeout.DEFAULT, recorder);
             this.transport.start();
 
             this.shutdownHook = new Thread(this::stop, "serve-shutdown");

serve-example/pom.xml

@@ -100,6 +100,16 @@
             <artifactId>base-json</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>build.base</groupId>
+            <artifactId>base-telemetry</artifactId>
+        </dependency>
+
+        <dependency>
+            <groupId>build.base</groupId>
+            <artifactId>base-telemetry-foundation</artifactId>
+        </dependency>
+
 
         <!-- Test -->
         <dependency>
@@ -175,6 +185,11 @@
                     <ignoredUnusedDeclaredDependencies>
                         <ignoredUnusedDeclaredDependency>build.base:base-template-processor</ignoredUnusedDeclaredDependency>
                     </ignoredUnusedDeclaredDependencies>
+                    <!-- Only referenced (NoOpTelemetryRecorder) from test code, but must be compile-scoped
+                         because module-info.java requires it for the main sourceset to compile. -->
+                    <ignoredNonTestScopedDependencies>
+                        <ignoredNonTestScopedDependency>build.base:base-telemetry-foundation</ignoredNonTestScopedDependency>
+                    </ignoredNonTestScopedDependencies>
                 </configuration>
             </plugin>
         </plugins>

serve-example/src/main/java/build/serve/example/ExampleApp.java

@@ -95,7 +95,7 @@ protected Router configure() {
         final var service = new TaskService();
         final var broadcaster = new TaskBroadcaster(service);
         final var api = new TaskApiHandler(service);
-        final var graphql = new TaskGraphQlHandler(service);
+        final var graphql = new TaskGraphQlHandler(service, recorder());
         final var web = new TaskWebHandler(service);
 
         final var apiAuth = AuthMiddleware.builder()

serve-example/src/main/java/build/serve/example/graphql/TaskGraphQlHandler.java

@@ -19,6 +19,7 @@
  */
 package build.serve.example.graphql;
 
+import build.base.telemetry.TelemetryRecorder;
 import build.serve.example.domain.TaskService;
 import build.serve.foundation.Handler;
 import build.serve.graphql.GraphQlHandler;
@@ -73,8 +74,9 @@ public final class TaskGraphQlHandler {
     /**
      * Constructs a {@link TaskGraphQlHandler} backed by the given service.
      */
-    public TaskGraphQlHandler(final TaskService service) {
+    public TaskGraphQlHandler(final TaskService service, final TelemetryRecorder recorder) {
         final var schema = GraphQlSchema.builder(SDL)
+            .recorder(recorder)
             .fetcher("Query", "tasks", env -> service.list())
             .fetcher("Query", "task", env -> {
                 final var id = Long.parseLong(env.<String>getArgument("id"));

serve-example/src/main/java/module-info.java

@@ -42,6 +42,8 @@
     requires build.base.network;
     requires build.base.template;
     requires build.base.json;
+    requires build.base.telemetry;
+    requires build.base.telemetry.foundation;
 
     requires static build.base.template.processor;
 

serve-example/src/test/java/build/serve/example/TaskGraphQlTests.java

@@ -22,6 +22,7 @@
 import build.base.json.Json;
 import build.base.json.JsonArray;
 import build.base.json.JsonNull;
+import build.base.telemetry.foundation.NoOpTelemetryRecorder;
 import build.serve.example.domain.TaskService;
 import build.serve.example.graphql.TaskGraphQlHandler;
 import build.serve.foundation.routing.RouterBuilder;
@@ -34,7 +35,7 @@ class TaskGraphQlTests {
 
     private static TestServer server() {
         final var service = new TaskService();
-        final var graphql = new TaskGraphQlHandler(service);
+        final var graphql = new TaskGraphQlHandler(service, NoOpTelemetryRecorder.create());
         return TestServer.of(RouterBuilder.create()
             .post("/graphql", graphql.graphqlHandler())
             .build());

serve-foundation/src/main/java/build/serve/foundation/option/RequestTimeout.java

@@ -41,6 +41,13 @@ public record RequestTimeout(Duration duration)
      */
     public static final RequestTimeout NONE = new RequestTimeout(Duration.ZERO);
 
+    /**
+     * A sane default request timeout (60 seconds), used by {@code ServerApplication} so a default
+     * server isn't exposed to Slowloris-style slow-request attacks. Pass {@link #NONE} explicitly
+     * to opt out.
+     */
+    public static final RequestTimeout DEFAULT = new RequestTimeout(Duration.ofSeconds(60));
+
     /**
      * Creates a {@link RequestTimeout} for the specified duration.
      *

serve-graphql/pom.xml

@@ -32,6 +32,16 @@
             <artifactId>base-json</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>build.base</groupId>
+            <artifactId>base-telemetry</artifactId>
+        </dependency>
+
+        <dependency>
+            <groupId>build.base</groupId>
+            <artifactId>base-telemetry-foundation</artifactId>
+        </dependency>
+
         <!-- Test Dependencies -->
         <dependency>
             <groupId>build.serve</groupId>

serve-graphql/src/main/java/build/serve/graphql/GraphQlSchema.java

@@ -19,9 +19,13 @@
  */
 package build.serve.graphql;
 
+import build.base.telemetry.TelemetryRecorder;
+import build.base.telemetry.foundation.PrintStreamTelemetryRecorder;
+import graphql.ExceptionWhileDataFetching;
 import graphql.ExecutionInput;
 import graphql.ExecutionResult;
 import graphql.GraphQL;
+import graphql.GraphQLError;
 import graphql.analysis.MaxQueryComplexityInstrumentation;
 import graphql.analysis.MaxQueryDepthInstrumentation;
 import graphql.execution.instrumentation.ChainedInstrumentation;
@@ -31,6 +35,7 @@
 import graphql.schema.idl.SchemaParser;
 import graphql.schema.idl.TypeDefinitionRegistry;
 
+import java.net.URI;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -47,10 +52,15 @@
  */
 public final class GraphQlSchema {
 
+    private static final TelemetryRecorder DEFAULT_RECORDER =
+        PrintStreamTelemetryRecorder.of(URI.create("serve://graphql"), System.out, System.err);
+
     private final GraphQL graphQL;
+    private final TelemetryRecorder recorder;
 
-    private GraphQlSchema(final GraphQL graphQL) {
+    private GraphQlSchema(final GraphQL graphQL, final TelemetryRecorder recorder) {
         this.graphQL = graphQL;
+        this.recorder = recorder;
     }
 
     /**
@@ -76,25 +86,44 @@ public GraphQlResult execute(final GraphQlRequest request) {
         final ExecutionResult result = graphQL.execute(inputBuilder.build());
 
         final List<GraphQlError> errors = result.getErrors().stream()
-            .map(e -> new GraphQlError(
-                sanitizeMessage(e.getMessage()),
-                e.getPath() != null
-                    ? e.getPath().stream().map(Object::toString).toList()
-                    : null))
+            .map(this::toGraphQlError)
             .toList();
 
         return new GraphQlResult(result.getData(), errors);
     }
 
     /**
-     * Strips control characters from a data-fetcher exception message before it reaches the
-     * client. graphql-java's default exception handler forwards a thrown exception's
-     * {@code getMessage()} verbatim into the error response; this matches the same
-     * control-character stripping {@code DefaultErrorHandler} and {@code McpServer} apply to
-     * exception messages at the HTTP and MCP layers, defending against CRLF/control-character
-     * injection via a data fetcher's exception message.
+     * Converts a graphql-java {@link GraphQLError} into a client-facing {@link GraphQlError}.
+     * <p>
+     * A {@link ExceptionWhileDataFetching} means a data fetcher threw — its message is an
+     * exception message, not a spec-defined client-facing error, and may contain internal details
+     * (stack state, SQL, file paths). It is logged server-side via {@link #recorder} and replaced
+     * with a generic message, matching the pattern {@code DefaultErrorHandler} applies to unhandled
+     * HTTP exceptions. Other error types (validation errors, syntax errors) are part of the GraphQL
+     * spec's client-facing contract and are passed through, only stripped of control characters to
+     * defend against CRLF/control-character injection.
+     *
+     * @param error the {@link GraphQLError} returned by the execution engine
+     * @return the client-facing {@link GraphQlError}
+     */
+    private GraphQlError toGraphQlError(final GraphQLError error) {
+        final var path = error.getPath() != null
+            ? error.getPath().stream().map(Object::toString).toList()
+            : null;
+
+        if (error instanceof ExceptionWhileDataFetching dataFetchingError) {
+            recorder.error(dataFetchingError.getException(), "Unhandled exception in GraphQL data fetcher");
+            return new GraphQlError("Internal Server Error", path);
+        }
+
+        return new GraphQlError(sanitizeMessage(error.getMessage()), path);
+    }
+
+    /**
+     * Strips control characters from a client-facing error message, defending against
+     * CRLF/control-character injection.
      *
-     * @param message the raw exception message, or {@code null}
+     * @param message the raw message, or {@code null}
      * @return the sanitized message, or {@code null} if the input was {@code null}
      */
     private static String sanitizeMessage(final String message) {
@@ -127,7 +156,8 @@ GraphQlSchema withOptions(final GraphQlOptions options) {
         return new GraphQlSchema(
             GraphQL.newGraphQL(graphQL.getGraphQLSchema())
                 .instrumentation(new ChainedInstrumentation(instrumentations))
-                .build()
+                .build(),
+            recorder
         );
     }
 
@@ -151,12 +181,25 @@ public static final class Builder {
 
         private final String sdl;
         private final Map<String, Map<String, DataFetcher<?>>> fetchers;
+        private TelemetryRecorder recorder;
 
         private Builder(final String sdl) {
             this.sdl = Objects.requireNonNull(sdl, "sdl must not be null");
             this.fetchers = new HashMap<>();
         }
 
+        /**
+         * Sets the {@link TelemetryRecorder} used to record unhandled data-fetcher exceptions.
+         * Defaults to a {@code System.out}/{@code System.err} recorder if not set.
+         *
+         * @param recorder the {@link TelemetryRecorder} to use
+         * @return this builder
+         */
+        public Builder recorder(final TelemetryRecorder recorder) {
+            this.recorder = Objects.requireNonNull(recorder, "recorder must not be null");
+            return this;
+        }
+
         /**
          * Registers a {@link DataFetcher} for a specific type and field.
          *
@@ -197,7 +240,7 @@ public GraphQlSchema build() {
             final var schema = new SchemaGenerator()
                 .makeExecutableSchema(registry, wiringBuilder.build());
 
-            return new GraphQlSchema(GraphQL.newGraphQL(schema).build());
+            return new GraphQlSchema(GraphQL.newGraphQL(schema).build(), recorder != null ? recorder : DEFAULT_RECORDER);
         }
 
         private static graphql.schema.DataFetcher<?> adaptFetcher(final DataFetcher<?> fetcher) {

serve-graphql/src/main/java/module-info.java

@@ -26,6 +26,7 @@
 module build.serve.graphql {
     requires transitive build.serve.foundation;
     requires build.base.json;
+    requires build.base.telemetry.foundation;
     requires com.graphqljava;
 
     exports build.serve.graphql;

serve-graphql/src/test/java/build/serve/graphql/GraphQlHandlerTests.java

@@ -139,14 +139,14 @@ void shouldReturnErrorsForInvalidQuery() throws Exception {
     }
 
     @Test
-    void shouldStripControlCharactersFromDataFetcherExceptionMessage() throws Exception {
+    void shouldNotLeakDataFetcherExceptionMessageToClient() throws Exception {
         final var throwingSchema = GraphQlSchema.builder("""
                 type Query {
                     boom: String
                 }
                 """)
             .fetcher("Query", "boom", env -> {
-                throw new RuntimeException("line one\r\nline two\tinjected");
+                throw new RuntimeException("line one\r\nline two\tinjected secret internal details");
             })
             .build();
 
@@ -165,11 +165,25 @@ void shouldStripControlCharactersFromDataFetcherExceptionMessage() throws Except
             final var errors = (JsonArray) json.get("errors");
             final var message = errors.values().get(0).asObject().getString("message");
 
-            assertThat(message).doesNotContain("\r").doesNotContain("\n").doesNotContain("\t");
-            assertThat(message).contains("line one  line two injected");
+            assertThat(message).isEqualTo("Internal Server Error");
         }
     }
 
+    @Test
+    void shouldStripControlCharactersFromSyntaxErrorMessage() throws Exception {
+        final var response = server.post("/graphql")
+            .header("Content-Type", "application/json")
+            .body("{\"query\":\"{ \\u0001 }\"}")
+            .send()
+            .assertStatus(200);
+
+        final var json = Json.parse(response.body()).asObject();
+        final var errors = (JsonArray) json.get("errors");
+        final var message = errors.values().get(0).asObject().getString("message");
+
+        assertThat(message).doesNotContain("\r").doesNotContain("\n").doesNotContain("\t");
+    }
+
     @Test
     void shouldBlockIntrospectionWhenDisabled() throws Exception {
         var schema = GraphQlSchema.builder("""