diff --git a/docs/cli.md b/docs/cli.md index fe12565..741a161 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -84,11 +84,14 @@ Notes: - `--profile-dir `: custom profile definition directory. - `--schema-dir `: schema base directory for `json_schema` validation. - `--artifact-root `: base directory for HTTP `file` input references. +- `--max-request-bytes `: maximum HTTP request body bytes; `0` disables this limit. +- `--max-artifact-bytes `: maximum HTTP file artifact bytes; `0` disables this limit. +- `--max-response-bytes `: maximum encoded HTTP response body bytes; `0` disables this limit. Notes: - `serve` does not accept runtime model override flags such as `--model` or `--llm-base-url`. - HTTP `file` input references are rejected unless an artifact root is configured through `server.artifact_root` or `--artifact-root`. -- `--artifact-root` affects only `serve`; `run` and `render` file input paths are unchanged. +- `--artifact-root` and the HTTP size-limit flags affect only `serve`; `run` and `render` file input paths are unchanged. ## Input And Variable Syntax diff --git a/docs/config.md b/docs/config.md index 2c88438..fd2b6b2 100644 --- a/docs/config.md +++ b/docs/config.md @@ -35,6 +35,9 @@ schema_dir: /opt/scriptorium/schemas server: addr: 127.0.0.1:8080 artifact_root: /var/lib/scriptorium/artifacts + max_request_bytes: 16777216 + max_artifact_bytes: 16777216 + max_response_bytes: 16777216 defaults: render_format: text @@ -49,6 +52,9 @@ Top-level fields: - `schema_dir` (optional): base directory for schema files used by `json_schema` validation. - `server.addr` (optional): default listen address for `serve`. - `server.artifact_root` (optional): base directory for HTTP `file` input references. +- `server.max_request_bytes` (optional): maximum HTTP request body size. `0` disables this limit. +- `server.max_artifact_bytes` (optional): maximum HTTP `file` input artifact size. `0` disables this limit. +- `server.max_response_bytes` (optional): maximum encoded HTTP response body size. `0` disables this limit. - `defaults.render_format` (optional): default `render` output format (`text` or `json`). Built-in defaults: @@ -56,11 +62,15 @@ Built-in defaults: - `schema_dir`: `.` - `server.addr`: `:8080` - `server.artifact_root`: unset; HTTP `file` input references are rejected until configured. +- `server.max_request_bytes`: `16777216` (16 MiB) +- `server.max_artifact_bytes`: `16777216` (16 MiB) +- `server.max_response_bytes`: `16777216` (16 MiB) - `defaults.render_format`: `text` Validation behavior: - Config decoding is strict; unknown YAML fields are rejected. +- HTTP size limit values must be greater than or equal to `0`. - Raw API key fields are not supported in `config.yml`. HTTP artifact root behavior: @@ -72,6 +82,13 @@ HTTP artifact root behavior: - Symlinks inside the root are followed by the operating system; do not make the artifact root writable by untrusted users. - CLI `run` and `render` file inputs keep their normal direct filesystem path behavior. +HTTP size-limit behavior: + +- The request limit covers the encoded JSON request body, including inline input bodies. +- The artifact limit covers HTTP `file` input artifacts read through `serve`. +- The response limit covers the final encoded JSON response, including generated artifact bodies and `raw_model_output` when requested. +- Limits apply only to HTTP `serve`; CLI `run` and `render` keep direct filesystem behavior. + ## Prompt Definition Files Prompt definitions are YAML files anywhere under `prompt_dir`, including nested subdirectories. diff --git a/docs/integrations/http-api.md b/docs/integrations/http-api.md index e3578e5..020dabb 100644 --- a/docs/integrations/http-api.md +++ b/docs/integrations/http-api.md @@ -75,6 +75,8 @@ Relative file URIs resolve inside that root. Absolute file URIs are accepted only when they remain inside the root. Requests that escape the root, including `..` traversal and absolute paths outside the root, return `400 artifact_not_allowed`. `inline` references do not require an artifact root. +HTTP file artifacts above the configured artifact limit return +`413 artifact_too_large`. Inline bodies are bounded by the request body limit. Model override notes: @@ -91,6 +93,8 @@ Request decoding uses strict JSON field checks: - unknown request fields are rejected with `400 invalid_json` - unknown `model` fields are rejected with `400 invalid_json` - raw API-key payload fields such as `api_key` are rejected as unknown fields +- request bodies above the configured request limit are rejected with `413 request_too_large` +- trailing JSON tokens after the request object are rejected with `400 invalid_json` ## Success Response @@ -204,6 +208,9 @@ Current error mapping (non-exhaustive): - `400 artifact_read_failed`: input artifact loading failed - `400 prompt_render_failed`: template render failed - `400 api_key_env_missing`: named API-key environment variable is missing +- `413 request_too_large`: request body exceeds the configured request limit +- `413 artifact_too_large`: HTTP file input artifact exceeds the configured artifact limit +- `413 response_too_large`: encoded JSON response exceeds the configured response limit - `404 prompt_not_found` - `404 profile_not_found` - `502 llm_failed`: outbound model request failed diff --git a/docs/internal/adapters.md b/docs/internal/adapters.md index e34c247..675401a 100644 --- a/docs/internal/adapters.md +++ b/docs/internal/adapters.md @@ -88,6 +88,9 @@ Primary app settings consumed by adapters: - `schema_dir` - `server.addr` - `server.artifact_root` (HTTP `serve` file input root) +- `server.max_request_bytes` +- `server.max_artifact_bytes` +- `server.max_response_bytes` - `defaults.render_format` Execution profile/request settings used through runner: @@ -122,6 +125,7 @@ Artifact refs: - Unsupported types return `ErrUnsupportedRefType`. - CLI `run` and `render` use direct filesystem file reads for `file` references. - HTTP `serve` uses a restricted artifact reader: `inline` references work without a root, while `file` references require `server.artifact_root` or `--artifact-root` and must stay inside that root. +- HTTP `serve` applies request-body, file-artifact, and encoded-response size limits. CLI `run` and `render` do not use these HTTP limits. - HTTP file paths are resolved with clean absolute paths and containment checks, not string-prefix checks. - Symlinks inside the root are followed by the operating system; the configured root must not be writable by untrusted users. @@ -149,6 +153,7 @@ Validator: HTTP error mapping: - maps domain/use-case errors to stable HTTP code + error code/message. +- maps request, artifact, and response size failures to `413` errors. - distinguishes missing profile selection and missing `api_key_env` variable using stable use-case sentinel errors. - avoids returning internal wrapped-cause details in response payload. diff --git a/docs/operations.md b/docs/operations.md index f39b179..120cba2 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -36,6 +36,9 @@ Built-in defaults relevant to operations: - `schema_dir: .` - `server.addr: :8080` - `server.artifact_root`: unset; HTTP `file` input references are rejected until configured +- `server.max_request_bytes: 16777216` +- `server.max_artifact_bytes: 16777216` +- `server.max_response_bytes: 16777216` - `defaults.render_format: text` ## Normal CLI Workflow @@ -78,6 +81,7 @@ Current inbound API behavior: - Validation content failures still return `200 OK` with `validation.status: "failed"`. - `inline` input references work without filesystem configuration. - `file` input references require `server.artifact_root` or `serve --artifact-root`; relative paths resolve inside that root and paths outside it are rejected. +- Request bodies, HTTP file input artifacts, and encoded JSON responses are limited by `server.max_request_bytes`, `server.max_artifact_bytes`, and `server.max_response_bytes`. Security caveat: @@ -85,6 +89,13 @@ Security caveat: - Deploy only behind trusted controls (private network boundary, authenticated reverse proxy, API gateway, or equivalent). - Keep the HTTP artifact root as narrow as practical and do not make it writable by untrusted users. +Sizing guidance: + +- Keep limits at the defaults unless a deployment has a measured need for larger prompt inputs or outputs. +- Prefer `inline` inputs for small payloads and HTTP `file` inputs for larger local artifacts inside a controlled artifact root. +- Increase the response limit when prompts intentionally return large generated artifacts or when clients request `include_raw_output`. +- Set a limit to `0` only for trusted deployments where another layer enforces request and response size. + ## Output, Logs, And Exit Codes `run` command: diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 092be81..c824ff1 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -147,6 +147,7 @@ Symptom: - CLI run/render error reading input artifacts. - HTTP `400 artifact_read_failed`. - HTTP `400 artifact_not_allowed`. +- HTTP `413 artifact_too_large`. Likely cause: @@ -154,12 +155,14 @@ Likely cause: - Unsupported artifact reference type in HTTP request. - HTTP `file` input references are disabled because no artifact root is configured. - HTTP `file` input path escapes the configured artifact root. +- HTTP `file` input artifact exceeds `server.max_artifact_bytes`. Diagnostic step: - Verify every mapped file path exists and is readable by the process. - For HTTP, verify each input uses supported `type` values. - For HTTP `file` inputs, verify `server.artifact_root` or `serve --artifact-root` is configured and the requested path stays inside that root. +- For HTTP `file` inputs, compare file size to `server.max_artifact_bytes`. Safe fix: @@ -167,6 +170,7 @@ Safe fix: - Use supported input types (`file`, `inline`). - Configure a narrow HTTP artifact root when HTTP file inputs are required. - Use relative paths under the artifact root, or switch to `inline` inputs. +- Increase `server.max_artifact_bytes` only when the deployment expects larger file inputs. Relevant links: @@ -351,22 +355,29 @@ Relevant links: Symptom: - HTTP `400 invalid_json` or `400 invalid_request`. +- HTTP `413 request_too_large`. +- HTTP `413 response_too_large`. Likely cause: - Malformed JSON body. - Unknown JSON fields. - Missing required `prompt_id` or `inputs`. +- Request body exceeds `server.max_request_bytes`, including inline input bodies. +- Encoded JSON response exceeds `server.max_response_bytes`, including generated artifact body and optional raw model output. Diagnostic step: - Revalidate request JSON. - Confirm required request fields are present. +- Compare request and expected response sizes to configured HTTP limits. Safe fix: - Send valid JSON with only supported fields. - Ensure `prompt_id` and at least one input mapping are included. +- Use smaller inline inputs, move large local inputs under the artifact root, or increase `server.max_request_bytes`. +- Omit `include_raw_output`, reduce generated output size, or increase `server.max_response_bytes`. Relevant links: diff --git a/internal/adapter/cli/run.go b/internal/adapter/cli/run.go index 6aa8568..b07ed72 100644 --- a/internal/adapter/cli/run.go +++ b/internal/adapter/cli/run.go @@ -74,11 +74,14 @@ type renderConfig struct { type serveConfig struct { configPath string - addr string - promptDir string - profileDir string - schemaDir string - artifactRoot string + addr string + promptDir string + profileDir string + schemaDir string + artifactRoot string + maxRequestBytes int64 + maxArtifactBytes int64 + maxResponseBytes int64 } type commonCommandSettings struct { @@ -87,6 +90,9 @@ type commonCommandSettings struct { schemaDir string serverAddr string artifactRoot string + maxRequestBytes int64 + maxArtifactBytes int64 + maxResponseBytes int64 defaultRenderFormat renderformat.PreparedRunOutputFormat } @@ -204,7 +210,7 @@ func serveCommand(args []string, stderr io.Writer) int { return ExitRuntimeError } - artifactReader, err := artifactadapter.NewRestrictedCompositeReader(cfg.artifactRoot) + artifactReader, err := artifactadapter.NewRestrictedCompositeReaderWithLimit(cfg.artifactRoot, cfg.maxArtifactBytes) if err != nil { fmt.Fprintf(stderr, "artifact root error: %v\n", err) return ExitRuntimeError @@ -212,7 +218,10 @@ func serveCommand(args []string, stderr io.Writer) int { runner := newRunnerWithArtifactReader(cfg.promptDir, cfg.profileDir, cfg.schemaDir, llmClient, artifactReader) - h := httpadapter.NewHandler(runner) + h := httpadapter.NewHandlerWithOptions(runner, httpadapter.HandlerOptions{ + MaxRequestBytes: cfg.maxRequestBytes, + MaxResponseBytes: cfg.maxResponseBytes, + }) srv := &http.Server{ Addr: cfg.addr, Handler: h, @@ -290,6 +299,9 @@ func parseServeArgs(args []string) (*serveConfig, error) { fs.StringVar(&cfg.profileDir, "profile-dir", "", "directory containing execution profile YAML files") fs.StringVar(&cfg.schemaDir, "schema-dir", "", "base directory for validation schemas") fs.StringVar(&cfg.artifactRoot, "artifact-root", "", "base directory for HTTP file input artifacts") + fs.Int64Var(&cfg.maxRequestBytes, "max-request-bytes", 0, "maximum HTTP request body bytes; 0 disables the limit") + fs.Int64Var(&cfg.maxArtifactBytes, "max-artifact-bytes", 0, "maximum HTTP file artifact bytes; 0 disables the limit") + fs.Int64Var(&cfg.maxResponseBytes, "max-response-bytes", 0, "maximum HTTP response body bytes; 0 disables the limit") if err := fs.Parse(args); err != nil { return nil, err @@ -299,11 +311,14 @@ func parseServeArgs(args []string) (*serveConfig, error) { } settings, err := resolveCommonSettings(fs, cfg.configPath, appconfig.CLIOverrides{ - PromptDir: cfg.promptDirIfSet(fs), - ProfileDir: cfg.profileDirIfSet(fs), - SchemaDir: cfg.schemaDirIfSet(fs), - ServerAddr: cfg.addrIfSet(fs), - ArtifactRoot: cfg.artifactRootIfSet(fs), + PromptDir: cfg.promptDirIfSet(fs), + ProfileDir: cfg.profileDirIfSet(fs), + SchemaDir: cfg.schemaDirIfSet(fs), + ServerAddr: cfg.addrIfSet(fs), + ArtifactRoot: cfg.artifactRootIfSet(fs), + MaxRequestBytes: cfg.maxRequestBytesIfSet(fs), + MaxArtifactBytes: cfg.maxArtifactBytesIfSet(fs), + MaxResponseBytes: cfg.maxResponseBytesIfSet(fs), }) if err != nil { return nil, err @@ -314,6 +329,9 @@ func parseServeArgs(args []string) (*serveConfig, error) { cfg.schemaDir = settings.schemaDir cfg.addr = settings.serverAddr cfg.artifactRoot = settings.artifactRoot + cfg.maxRequestBytes = settings.maxRequestBytes + cfg.maxArtifactBytes = settings.maxArtifactBytes + cfg.maxResponseBytes = settings.maxResponseBytes if err := validateRequiredLibraryDirs(cfg.promptDir); err != nil { return nil, err @@ -450,6 +468,27 @@ func (c *serveConfig) artifactRootIfSet(fs *flag.FlagSet) string { return "" } +func (c *serveConfig) maxRequestBytesIfSet(fs *flag.FlagSet) *int64 { + if flagWasSet(fs, "max-request-bytes") { + return &c.maxRequestBytes + } + return nil +} + +func (c *serveConfig) maxArtifactBytesIfSet(fs *flag.FlagSet) *int64 { + if flagWasSet(fs, "max-artifact-bytes") { + return &c.maxArtifactBytes + } + return nil +} + +func (c *serveConfig) maxResponseBytesIfSet(fs *flag.FlagSet) *int64 { + if flagWasSet(fs, "max-response-bytes") { + return &c.maxResponseBytes + } + return nil +} + func registerConfigPathFlag(fs *flag.FlagSet, target *string) { fs.StringVar( target, @@ -488,6 +527,9 @@ func resolveCommonSettings(fs *flag.FlagSet, configPath string, overrides appcon schemaDir: settings.SchemaDir, serverAddr: settings.ServerAddr, artifactRoot: settings.ArtifactRoot, + maxRequestBytes: settings.MaxRequestBytes, + maxArtifactBytes: settings.MaxArtifactBytes, + maxResponseBytes: settings.MaxResponseBytes, defaultRenderFormat: settings.DefaultRenderFormat, }, nil } diff --git a/internal/adapter/cli/run_test.go b/internal/adapter/cli/run_test.go index cc141e7..7a37bd7 100644 --- a/internal/adapter/cli/run_test.go +++ b/internal/adapter/cli/run_test.go @@ -436,12 +436,18 @@ schema_dir: ./from-config/schemas server: addr: 127.0.0.1:9000 artifact_root: ./from-config/artifacts + max_request_bytes: 1024 + max_artifact_bytes: 2048 + max_response_bytes: 4096 `) cfg, err := parseServeArgs([]string{ "--config", configPath, "--addr", ":7777", "--artifact-root", "./from-cli/artifacts", + "--max-request-bytes", "0", + "--max-artifact-bytes", "8192", + "--max-response-bytes", "16384", }) if err != nil { t.Fatalf("expected valid args, got %v", err) @@ -462,6 +468,15 @@ server: if cfg.artifactRoot != filepath.Clean("./from-cli/artifacts") { t.Fatalf("expected CLI artifact root override, got %q", cfg.artifactRoot) } + if cfg.maxRequestBytes != 0 { + t.Fatalf("expected CLI max request bytes override, got %d", cfg.maxRequestBytes) + } + if cfg.maxArtifactBytes != 8192 { + t.Fatalf("expected CLI max artifact bytes override, got %d", cfg.maxArtifactBytes) + } + if cfg.maxResponseBytes != 16384 { + t.Fatalf("expected CLI max response bytes override, got %d", cfg.maxResponseBytes) + } } func TestParseServeArgsWithConfigProvidesRequiredDirectoriesAndAddr(t *testing.T) { @@ -472,6 +487,9 @@ schema_dir: ./from-config/schemas server: addr: 127.0.0.1:9000 artifact_root: ./from-config/artifacts + max_request_bytes: 1024 + max_artifact_bytes: 2048 + max_response_bytes: 4096 `) cfg, err := parseServeArgs([]string{ @@ -496,6 +514,72 @@ server: if cfg.artifactRoot != filepath.Clean("./from-config/artifacts") { t.Fatalf("expected artifact root from config, got %q", cfg.artifactRoot) } + if cfg.maxRequestBytes != 1024 { + t.Fatalf("expected max request bytes from config, got %d", cfg.maxRequestBytes) + } + if cfg.maxArtifactBytes != 2048 { + t.Fatalf("expected max artifact bytes from config, got %d", cfg.maxArtifactBytes) + } + if cfg.maxResponseBytes != 4096 { + t.Fatalf("expected max response bytes from config, got %d", cfg.maxResponseBytes) + } +} + +func TestParseServeArgsRejectsNegativeSizeLimits(t *testing.T) { + tests := []struct { + name string + flag string + }{ + {name: "request", flag: "--max-request-bytes"}, + {name: "artifact", flag: "--max-artifact-bytes"}, + {name: "response", flag: "--max-response-bytes"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := parseServeArgs([]string{ + "--prompt-dir", "./prompts", + tc.flag, "-1", + }) + if err == nil { + t.Fatal("expected negative size limit error") + } + }) + } +} + +func TestRunAndRenderRejectServeSizeLimitFlags(t *testing.T) { + for _, tc := range []struct { + name string + parse func([]string) error + }{ + { + name: "run", + parse: func(args []string) error { + _, err := parseRunArgs(args) + return err + }, + }, + { + name: "render", + parse: func(args []string) error { + _, err := parseRenderArgs(args) + return err + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + err := tc.parse([]string{ + "--prompt-dir", "./prompts", + "--prompt", "p", + "--input", "a=b", + "--max-request-bytes", "1024", + }) + if err == nil { + t.Fatal("expected unsupported flag error") + } + }) + } } func TestRunAndRenderBuildEquivalentRuntimeOverrideRequestsForSharedFlags(t *testing.T) { diff --git a/internal/adapter/http/handler.go b/internal/adapter/http/handler.go index 5c06e04..2952779 100644 --- a/internal/adapter/http/handler.go +++ b/internal/adapter/http/handler.go @@ -4,10 +4,12 @@ import ( "context" "encoding/json" "errors" + "io" "net/http" "strings" "gitea.maximumdirect.net/eric/scriptorium/internal/artifact" + "gitea.maximumdirect.net/eric/scriptorium/internal/defaults" "gitea.maximumdirect.net/eric/scriptorium/internal/domain" "gitea.maximumdirect.net/eric/scriptorium/internal/profile" "gitea.maximumdirect.net/eric/scriptorium/internal/promptdef" @@ -19,11 +21,24 @@ type Runner interface { } type Handler struct { - runner Runner + runner Runner + options HandlerOptions +} + +type HandlerOptions struct { + MaxRequestBytes int64 + MaxResponseBytes int64 } func NewHandler(runner Runner) *Handler { - return &Handler{runner: runner} + return NewHandlerWithOptions(runner, HandlerOptions{ + MaxRequestBytes: defaults.HTTPMaxRequestBytesDefault, + MaxResponseBytes: defaults.HTTPMaxResponseBytesDefault, + }) +} + +func NewHandlerWithOptions(runner Runner, options HandlerOptions) *Handler { + return &Handler{runner: runner, options: options} } func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -37,9 +52,26 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } var req runRequestDTO - dec := json.NewDecoder(r.Body) + body := r.Body + if h.options.MaxRequestBytes > 0 { + body = http.MaxBytesReader(w, r.Body, h.options.MaxRequestBytes) + } + dec := json.NewDecoder(body) dec.DisallowUnknownFields() if err := dec.Decode(&req); err != nil { + if isRequestTooLarge(err) { + writeError(w, http.StatusRequestEntityTooLarge, "request_too_large", "request body is too large") + return + } + writeError(w, http.StatusBadRequest, "invalid_json", "invalid JSON request body") + return + } + var trailing any + if err := dec.Decode(&trailing); err != io.EOF { + if isRequestTooLarge(err) { + writeError(w, http.StatusRequestEntityTooLarge, "request_too_large", "request body is too large") + return + } writeError(w, http.StatusBadRequest, "invalid_json", "invalid JSON request body") return } @@ -121,7 +153,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { raw := res.RawOutput resp.RawModelOutput = &raw } - writeJSON(w, http.StatusOK, resp) + writeLimitedJSON(w, http.StatusOK, resp, h.options.MaxResponseBytes) } func executionTargetOverrideFromModelOverrideDTO(dto *modelOverrideRequestDTO) *domain.ExecutionTargetOverride { @@ -190,6 +222,8 @@ func mapRunError(err error) (int, string, string) { return http.StatusBadRequest, "profile_load_failed", "failed to load execution profile" case errors.Is(err, artifact.ErrFileNotAllowed), errors.Is(err, artifact.ErrFileOutsideRoot): return http.StatusBadRequest, "artifact_not_allowed", "file input artifact is not allowed" + case errors.Is(err, artifact.ErrFileTooLarge): + return http.StatusRequestEntityTooLarge, "artifact_too_large", "file input artifact is too large" case errors.Is(err, usecase.ErrArtifactLoad): return http.StatusBadRequest, "artifact_read_failed", "failed to read input artifact" case errors.Is(err, usecase.ErrPromptRender): @@ -204,9 +238,23 @@ func mapRunError(err error) (int, string, string) { } func writeJSON(w http.ResponseWriter, status int, v any) { + writeLimitedJSON(w, status, v, 0) +} + +func writeLimitedJSON(w http.ResponseWriter, status int, v any, maxBytes int64) { + data, err := json.Marshal(v) + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "internal server error") + return + } + data = append(data, '\n') + if maxBytes > 0 && int64(len(data)) > maxBytes { + writeError(w, http.StatusRequestEntityTooLarge, "response_too_large", "response body is too large") + return + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) - _ = json.NewEncoder(w).Encode(v) + _, _ = w.Write(data) } func writeError(w http.ResponseWriter, status int, code, message string) { @@ -217,3 +265,8 @@ func writeError(w http.ResponseWriter, status int, code, message string) { }, }) } + +func isRequestTooLarge(err error) bool { + var maxBytesErr *http.MaxBytesError + return errors.As(err, &maxBytesErr) +} diff --git a/internal/adapter/http/handler_test.go b/internal/adapter/http/handler_test.go index 6b60238..5d3dad7 100644 --- a/internal/adapter/http/handler_test.go +++ b/internal/adapter/http/handler_test.go @@ -243,6 +243,24 @@ func TestHandlerFileRefsUnderArtifactRootWork(t *testing.T) { } } +func TestHandlerFileRefsAboveArtifactLimitAreRejected(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "large.txt"), []byte("123456"), 0o644); err != nil { + t.Fatal(err) + } + h := newArtifactRootHandlerWithLimit(t, root, 5) + + req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString(`{ + "prompt_id":"p", + "inputs":{"x":{"type":"file","uri":"large.txt"}} + }`)) + w := httptest.NewRecorder() + + h.ServeHTTP(w, req) + + assertHTTPErrorCode(t, w, http.StatusRequestEntityTooLarge, "artifact_too_large") +} + func TestHandlerFileRefsOutsideArtifactRootAreRejected(t *testing.T) { root := t.TempDir() outside := t.TempDir() @@ -576,6 +594,69 @@ func TestHandlerInvalidJSON(t *testing.T) { } } +func TestHandlerRejectsTrailingJSON(t *testing.T) { + h := NewHandler(&fakeRunner{}) + req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString(`{"prompt_id":"p","inputs":{"x":{"type":"file","uri":"a"}}} {}`)) + w := httptest.NewRecorder() + + h.ServeHTTP(w, req) + + assertHTTPErrorCode(t, w, http.StatusBadRequest, "invalid_json") +} + +func TestHandlerRequestTooLarge(t *testing.T) { + h := NewHandlerWithOptions(&fakeRunner{}, HandlerOptions{MaxRequestBytes: 12}) + req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString(`{"prompt_id":"p","inputs":{"x":{"type":"file","uri":"a"}}}`)) + w := httptest.NewRecorder() + + h.ServeHTTP(w, req) + + assertHTTPErrorCode(t, w, http.StatusRequestEntityTooLarge, "request_too_large") +} + +func TestHandlerMalformedJSONBelowLimitStillBadRequest(t *testing.T) { + h := NewHandlerWithOptions(&fakeRunner{}, HandlerOptions{MaxRequestBytes: 1024}) + req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString("{")) + w := httptest.NewRecorder() + + h.ServeHTTP(w, req) + + assertHTTPErrorCode(t, w, http.StatusBadRequest, "invalid_json") +} + +func TestHandlerResponseTooLarge(t *testing.T) { + h := NewHandlerWithOptions(&fakeRunner{result: &domain.RunResult{ + Artifact: domain.Artifact{Body: []byte(strings.Repeat("x", 128))}, + Validation: domain.ValidationResult{Status: domain.ValidationPassed, Mode: domain.ValidationBasic, IsValid: true}, + EffectiveModelParams: domain.ExecutionTarget{Endpoint: "http://llm/v1", Model: "m1"}, + }}, HandlerOptions{MaxRequestBytes: 1024, MaxResponseBytes: 64}) + req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString(`{"prompt_id":"p","inputs":{"x":{"type":"file","uri":"a"}}}`)) + w := httptest.NewRecorder() + + h.ServeHTTP(w, req) + + assertHTTPErrorCode(t, w, http.StatusRequestEntityTooLarge, "response_too_large") +} + +func TestHandlerRawOutputDoesNotBypassResponseLimit(t *testing.T) { + h := NewHandlerWithOptions(&fakeRunner{result: &domain.RunResult{ + Artifact: domain.Artifact{Body: []byte("ok")}, + RawOutput: strings.Repeat("raw", 80), + Validation: domain.ValidationResult{Status: domain.ValidationPassed, Mode: domain.ValidationBasic, IsValid: true}, + EffectiveModelParams: domain.ExecutionTarget{Endpoint: "http://llm/v1", Model: "m1"}, + }}, HandlerOptions{MaxRequestBytes: 1024, MaxResponseBytes: 128}) + req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString(`{ + "prompt_id":"p", + "inputs":{"x":{"type":"file","uri":"a"}}, + "include_raw_output":true + }`)) + w := httptest.NewRecorder() + + h.ServeHTTP(w, req) + + assertHTTPErrorCode(t, w, http.StatusRequestEntityTooLarge, "response_too_large") +} + func TestHandlerMissingPromptID(t *testing.T) { h := NewHandler(&fakeRunner{}) req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString(`{"inputs":{"x":{"type":"file","uri":"a"}}}`)) @@ -770,7 +851,13 @@ func wrap(stage error, cause error) error { func newArtifactRootHandler(t *testing.T, root string) *Handler { t.Helper() - reader, err := artifact.NewRestrictedCompositeReader(root) + return newArtifactRootHandlerWithLimit(t, root, 0) +} + +func newArtifactRootHandlerWithLimit(t *testing.T, root string, maxArtifactBytes int64) *Handler { + t.Helper() + + reader, err := artifact.NewRestrictedCompositeReaderWithLimit(root, maxArtifactBytes) if err != nil { t.Fatalf("expected restricted artifact reader: %v", err) } diff --git a/internal/artifact/reader.go b/internal/artifact/reader.go index 7c70c2e..0b7fde3 100644 --- a/internal/artifact/reader.go +++ b/internal/artifact/reader.go @@ -7,6 +7,7 @@ import ( "fmt" "gitea.maximumdirect.net/eric/scriptorium/internal/defaults" "gitea.maximumdirect.net/eric/scriptorium/internal/domain" + "io" "mime" "os" "path/filepath" @@ -19,6 +20,7 @@ var ( ErrMissingFilePath = errors.New("missing file path for file artifact") ErrFileNotAllowed = errors.New("file artifact references are not allowed") ErrFileOutsideRoot = errors.New("file artifact path is outside artifact root") + ErrFileTooLarge = errors.New("file artifact exceeds size limit") ) // Reader resolves artifact references into actual artifacts. @@ -40,7 +42,11 @@ func NewCompositeReader() Reader { } func NewRestrictedCompositeReader(root string) (Reader, error) { - fileReader, err := newRestrictedFileReader(root) + return NewRestrictedCompositeReaderWithLimit(root, 0) +} + +func NewRestrictedCompositeReaderWithLimit(root string, maxBytes int64) (Reader, error) { + fileReader, err := newRestrictedFileReader(root, maxBytes) if err != nil { return nil, err } @@ -122,10 +128,14 @@ func (r deniedFileReader) Read(ctx context.Context, ref domain.ArtifactRef) (*do } type restrictedFileReader struct { - root string + root string + maxBytes int64 } -func newRestrictedFileReader(root string) (Reader, error) { +func newRestrictedFileReader(root string, maxBytes int64) (Reader, error) { + if maxBytes < 0 { + return nil, fmt.Errorf("artifact size limit must be greater than or equal to 0") + } cleanRoot := strings.TrimSpace(root) if cleanRoot == "" { return deniedFileReader{}, nil @@ -134,7 +144,7 @@ func newRestrictedFileReader(root string) (Reader, error) { if err != nil { return nil, fmt.Errorf("resolve artifact root: %w", err) } - return &restrictedFileReader{root: absRoot}, nil + return &restrictedFileReader{root: absRoot, maxBytes: maxBytes}, nil } func (r *restrictedFileReader) Read(ctx context.Context, ref domain.ArtifactRef) (*domain.Artifact, error) { @@ -152,7 +162,7 @@ func (r *restrictedFileReader) Read(ctx context.Context, ref domain.ArtifactRef) if err != nil { return nil, err } - return readFileArtifact(path) + return readFileArtifactWithLimit(path, r.maxBytes) } func (r *restrictedFileReader) resolve(rawPath string) (string, error) { @@ -181,10 +191,39 @@ func (r *restrictedFileReader) resolve(rawPath string) (string, error) { } func readFileArtifact(path string) (*domain.Artifact, error) { - data, err := os.ReadFile(path) + return readFileArtifactWithLimit(path, 0) +} + +func readFileArtifactWithLimit(path string, maxBytes int64) (*domain.Artifact, error) { + if maxBytes < 0 { + return nil, fmt.Errorf("file size limit must be greater than or equal to 0") + } + + file, err := os.Open(path) if err != nil { return nil, fmt.Errorf("failed to read file %s: %w", path, err) } + defer file.Close() + + info, err := file.Stat() + if err != nil { + return nil, fmt.Errorf("failed to stat file %s: %w", path, err) + } + if maxBytes > 0 && info.Size() > maxBytes { + return nil, ErrFileTooLarge + } + + var reader io.Reader = file + if maxBytes > 0 { + reader = io.LimitReader(file, maxBytes+1) + } + data, err := io.ReadAll(reader) + if err != nil { + return nil, fmt.Errorf("failed to read file %s: %w", path, err) + } + if maxBytes > 0 && int64(len(data)) > maxBytes { + return nil, ErrFileTooLarge + } contentType := mime.TypeByExtension(filepath.Ext(path)) if contentType == "" { diff --git a/internal/artifact/reader_test.go b/internal/artifact/reader_test.go index 7b8cff0..e1cb298 100644 --- a/internal/artifact/reader_test.go +++ b/internal/artifact/reader_test.go @@ -132,6 +132,54 @@ func TestRestrictedCompositeReaderWithoutRootDeniesFileRefs(t *testing.T) { } } +func TestRestrictedCompositeReaderFileSizeLimit(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "exact.txt"), []byte("12345"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "large.txt"), []byte("123456"), 0o644); err != nil { + t.Fatal(err) + } + + reader, err := NewRestrictedCompositeReaderWithLimit(root, 5) + if err != nil { + t.Fatalf("expected restricted reader construction, got %v", err) + } + + art, err := reader.Read(ctx, domain.ArtifactRef{Type: domain.ArtifactRefFile, URI: "exact.txt"}) + if err != nil { + t.Fatalf("expected file at limit to succeed, got %v", err) + } + if string(art.Body) != "12345" { + t.Fatalf("unexpected artifact body: %q", string(art.Body)) + } + + _, err = reader.Read(ctx, domain.ArtifactRef{Type: domain.ArtifactRefFile, URI: "large.txt"}) + if !errors.Is(err, ErrFileTooLarge) { + t.Fatalf("expected ErrFileTooLarge, got %v", err) + } +} + +func TestRestrictedCompositeReaderFileSizeLimitZeroDisablesLimit(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "large.txt"), []byte("123456"), 0o644); err != nil { + t.Fatal(err) + } + + reader, err := NewRestrictedCompositeReaderWithLimit(root, 0) + if err != nil { + t.Fatalf("expected restricted reader construction, got %v", err) + } + art, err := reader.Read(context.Background(), domain.ArtifactRef{Type: domain.ArtifactRefFile, URI: "large.txt"}) + if err != nil { + t.Fatalf("expected unlimited reader to succeed, got %v", err) + } + if string(art.Body) != "123456" { + t.Fatalf("unexpected artifact body: %q", string(art.Body)) + } +} + func TestFileReader_Read(t *testing.T) { content := []byte("test file content") tmpFile, err := os.CreateTemp("", "artifact_test_*.txt") diff --git a/internal/config/config.go b/internal/config/config.go index 3334924..a9af376 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -40,8 +40,11 @@ type Config struct { } type ServerConfig struct { - Addr string `yaml:"addr"` - ArtifactRoot string `yaml:"artifact_root"` + Addr string `yaml:"addr"` + ArtifactRoot string `yaml:"artifact_root"` + MaxRequestBytes *int64 `yaml:"max_request_bytes"` + MaxArtifactBytes *int64 `yaml:"max_artifact_bytes"` + MaxResponseBytes *int64 `yaml:"max_response_bytes"` } type DefaultsConfig struct { @@ -55,17 +58,23 @@ type AppSettings struct { SchemaDir string ServerAddr string ArtifactRoot string + MaxRequestBytes int64 + MaxArtifactBytes int64 + MaxResponseBytes int64 DefaultRenderFormat renderformat.PreparedRunOutputFormat } // CLIOverrides can be applied after config load to enforce precedence. type CLIOverrides struct { - PromptDir string - ProfileDir string - SchemaDir string - ServerAddr string - ArtifactRoot string - RenderFormat string + PromptDir string + ProfileDir string + SchemaDir string + ServerAddr string + ArtifactRoot string + MaxRequestBytes *int64 + MaxArtifactBytes *int64 + MaxResponseBytes *int64 + RenderFormat string } // BuiltInDefaults returns compile-time application defaults. @@ -73,6 +82,9 @@ func BuiltInDefaults() AppSettings { return AppSettings{ SchemaDir: defaults.SchemaDirDefault, ServerAddr: defaults.HTTPAddrDefault, + MaxRequestBytes: defaults.HTTPMaxRequestBytesDefault, + MaxArtifactBytes: defaults.HTTPMaxArtifactBytesDefault, + MaxResponseBytes: defaults.HTTPMaxResponseBytesDefault, DefaultRenderFormat: renderformat.DefaultPreparedRunOutputFormat, } } @@ -148,6 +160,24 @@ func ApplyCLIOverrides(base AppSettings, overrides CLIOverrides) (AppSettings, e if v := strings.TrimSpace(overrides.ArtifactRoot); v != "" { out.ArtifactRoot = filepath.Clean(v) } + if overrides.MaxRequestBytes != nil { + if *overrides.MaxRequestBytes < 0 { + return AppSettings{}, fmt.Errorf("%w: server.max_request_bytes must be greater than or equal to 0", ErrInvalidConfig) + } + out.MaxRequestBytes = *overrides.MaxRequestBytes + } + if overrides.MaxArtifactBytes != nil { + if *overrides.MaxArtifactBytes < 0 { + return AppSettings{}, fmt.Errorf("%w: server.max_artifact_bytes must be greater than or equal to 0", ErrInvalidConfig) + } + out.MaxArtifactBytes = *overrides.MaxArtifactBytes + } + if overrides.MaxResponseBytes != nil { + if *overrides.MaxResponseBytes < 0 { + return AppSettings{}, fmt.Errorf("%w: server.max_response_bytes must be greater than or equal to 0", ErrInvalidConfig) + } + out.MaxResponseBytes = *overrides.MaxResponseBytes + } if rawFormat := strings.TrimSpace(overrides.RenderFormat); rawFormat != "" { parsed, err := renderformat.ParsePreparedRunOutputFormat(rawFormat) if err != nil { @@ -190,6 +220,24 @@ func applyConfig(base AppSettings, cfg Config) (AppSettings, error) { if v := strings.TrimSpace(cfg.Server.ArtifactRoot); v != "" { out.ArtifactRoot = filepath.Clean(v) } + if cfg.Server.MaxRequestBytes != nil { + if *cfg.Server.MaxRequestBytes < 0 { + return AppSettings{}, fmt.Errorf("%w: server.max_request_bytes must be greater than or equal to 0", ErrInvalidConfig) + } + out.MaxRequestBytes = *cfg.Server.MaxRequestBytes + } + if cfg.Server.MaxArtifactBytes != nil { + if *cfg.Server.MaxArtifactBytes < 0 { + return AppSettings{}, fmt.Errorf("%w: server.max_artifact_bytes must be greater than or equal to 0", ErrInvalidConfig) + } + out.MaxArtifactBytes = *cfg.Server.MaxArtifactBytes + } + if cfg.Server.MaxResponseBytes != nil { + if *cfg.Server.MaxResponseBytes < 0 { + return AppSettings{}, fmt.Errorf("%w: server.max_response_bytes must be greater than or equal to 0", ErrInvalidConfig) + } + out.MaxResponseBytes = *cfg.Server.MaxResponseBytes + } if rawFormat := strings.TrimSpace(cfg.Defaults.RenderFormat); rawFormat != "" { parsed, err := renderformat.ParsePreparedRunOutputFormat(rawFormat) if err != nil { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 1278846..0a54d1f 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "testing" + "gitea.maximumdirect.net/eric/scriptorium/internal/defaults" renderformat "gitea.maximumdirect.net/eric/scriptorium/internal/format" ) @@ -24,6 +25,20 @@ func TestLoadConfigMissingImplicitPathUsesBuiltInDefaults(t *testing.T) { } } +func TestBuiltInDefaultsIncludeHTTPSizeLimits(t *testing.T) { + got := BuiltInDefaults() + + if got.MaxRequestBytes != defaults.HTTPMaxRequestBytesDefault { + t.Fatalf("unexpected max request bytes: %d", got.MaxRequestBytes) + } + if got.MaxArtifactBytes != defaults.HTTPMaxArtifactBytesDefault { + t.Fatalf("unexpected max artifact bytes: %d", got.MaxArtifactBytes) + } + if got.MaxResponseBytes != defaults.HTTPMaxResponseBytesDefault { + t.Fatalf("unexpected max response bytes: %d", got.MaxResponseBytes) + } +} + func TestLoadConfigMissingExplicitPathReturnsError(t *testing.T) { tmp := t.TempDir() missing := filepath.Join(tmp, "missing.yml") @@ -93,6 +108,9 @@ schema_dir: ./schemas server: addr: 127.0.0.1:9090 artifact_root: ./artifacts + max_request_bytes: 1024 + max_artifact_bytes: 2048 + max_response_bytes: 4096 defaults: render_format: json `) @@ -117,11 +135,58 @@ defaults: if got.ArtifactRoot != filepath.Clean("./artifacts") { t.Fatalf("unexpected server.artifact_root: %q", got.ArtifactRoot) } + if got.MaxRequestBytes != 1024 { + t.Fatalf("unexpected server.max_request_bytes: %d", got.MaxRequestBytes) + } + if got.MaxArtifactBytes != 2048 { + t.Fatalf("unexpected server.max_artifact_bytes: %d", got.MaxArtifactBytes) + } + if got.MaxResponseBytes != 4096 { + t.Fatalf("unexpected server.max_response_bytes: %d", got.MaxResponseBytes) + } if got.DefaultRenderFormat != renderformat.PreparedRunFormatJSON { t.Fatalf("unexpected defaults.render_format: %q", got.DefaultRenderFormat) } } +func TestLoadConfigAcceptsZeroHTTPSizeLimits(t *testing.T) { + path := writeConfigFile(t, "config.yml", ` +server: + max_request_bytes: 0 + max_artifact_bytes: 0 + max_response_bytes: 0 +`) + + got, err := LoadConfig(path, true) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if got.MaxRequestBytes != 0 || got.MaxArtifactBytes != 0 || got.MaxResponseBytes != 0 { + t.Fatalf("expected zero limits to be preserved, got request=%d artifact=%d response=%d", got.MaxRequestBytes, got.MaxArtifactBytes, got.MaxResponseBytes) + } +} + +func TestLoadConfigRejectsNegativeHTTPSizeLimits(t *testing.T) { + tests := []struct { + name string + body string + }{ + {name: "request", body: "server:\n max_request_bytes: -1\n"}, + {name: "artifact", body: "server:\n max_artifact_bytes: -1\n"}, + {name: "response", body: "server:\n max_response_bytes: -1\n"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + path := writeConfigFile(t, "config.yml", tc.body) + _, err := LoadConfig(path, true) + if !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig, got %v", err) + } + }) + } +} + func TestLoadConfigEmptyFileResolvesToBuiltInDefaults(t *testing.T) { path := writeConfigFile(t, "config.yml", "") @@ -190,16 +255,25 @@ func TestApplyCLIOverridesAppliesPrecedence(t *testing.T) { SchemaDir: "/from/config/schemas", ServerAddr: ":1234", ArtifactRoot: "/from/config/artifacts", + MaxRequestBytes: 111, + MaxArtifactBytes: 222, + MaxResponseBytes: 333, DefaultRenderFormat: renderformat.PreparedRunFormatJSON, } + maxRequestBytes := int64(0) + maxArtifactBytes := int64(444) + maxResponseBytes := int64(555) got, err := ApplyCLIOverrides(base, CLIOverrides{ - PromptDir: "./prompts-cli", - ProfileDir: "./profiles-cli", - SchemaDir: "./schemas-cli", - ServerAddr: ":8081", - ArtifactRoot: "./artifacts-cli", - RenderFormat: "text", + PromptDir: "./prompts-cli", + ProfileDir: "./profiles-cli", + SchemaDir: "./schemas-cli", + ServerAddr: ":8081", + ArtifactRoot: "./artifacts-cli", + MaxRequestBytes: &maxRequestBytes, + MaxArtifactBytes: &maxArtifactBytes, + MaxResponseBytes: &maxResponseBytes, + RenderFormat: "text", }) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -220,11 +294,42 @@ func TestApplyCLIOverridesAppliesPrecedence(t *testing.T) { if got.ArtifactRoot != filepath.Clean("./artifacts-cli") { t.Fatalf("unexpected artifact root: %q", got.ArtifactRoot) } + if got.MaxRequestBytes != 0 { + t.Fatalf("unexpected max request bytes: %d", got.MaxRequestBytes) + } + if got.MaxArtifactBytes != 444 { + t.Fatalf("unexpected max artifact bytes: %d", got.MaxArtifactBytes) + } + if got.MaxResponseBytes != 555 { + t.Fatalf("unexpected max response bytes: %d", got.MaxResponseBytes) + } if got.DefaultRenderFormat != renderformat.PreparedRunFormatText { t.Fatalf("unexpected render format: %q", got.DefaultRenderFormat) } } +func TestApplyCLIOverridesRejectsNegativeHTTPSizeLimits(t *testing.T) { + negative := int64(-1) + + tests := []struct { + name string + overrides CLIOverrides + }{ + {name: "request", overrides: CLIOverrides{MaxRequestBytes: &negative}}, + {name: "artifact", overrides: CLIOverrides{MaxArtifactBytes: &negative}}, + {name: "response", overrides: CLIOverrides{MaxResponseBytes: &negative}}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := ApplyCLIOverrides(BuiltInDefaults(), tc.overrides) + if !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig, got %v", err) + } + }) + } +} + func TestApplyCLIOverridesInvalidRenderFormatReturnsError(t *testing.T) { _, err := ApplyCLIOverrides(BuiltInDefaults(), CLIOverrides{RenderFormat: "yaml"}) if err == nil { diff --git a/internal/defaults/defaults.go b/internal/defaults/defaults.go index c0d8c85..892b224 100644 --- a/internal/defaults/defaults.go +++ b/internal/defaults/defaults.go @@ -7,13 +7,16 @@ import ( ) const ( - HTTPAddrDefault = ":8080" - SchemaDirDefault = "." - OutputArtifactName = "output" - ContentTypeTextPlain = "text/plain" - ContentTypeTextMarkdown = "text/markdown" - ContentTypeApplicationJSON = "application/json" - OpenAIChatCompletionsPath = "/chat/completions" + HTTPAddrDefault = ":8080" + SchemaDirDefault = "." + OutputArtifactName = "output" + ContentTypeTextPlain = "text/plain" + ContentTypeTextMarkdown = "text/markdown" + ContentTypeApplicationJSON = "application/json" + OpenAIChatCompletionsPath = "/chat/completions" + HTTPMaxRequestBytesDefault = 16 * 1024 * 1024 + HTTPMaxArtifactBytesDefault = 16 * 1024 * 1024 + HTTPMaxResponseBytesDefault = 16 * 1024 * 1024 ExecutionDefaultTemperature = 0.0 ExecutionDefaultMaxTokens = 0