Refactor http adapter to enforce strict run request JSON, expand prompt/profile error mapping, and harden api_key handling

internal/adapter/http/handler.go

@@ -36,7 +36,9 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	var req runRequestDTO
-	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+	dec := json.NewDecoder(r.Body)
+	dec.DisallowUnknownFields()
+	if err := dec.Decode(&req); err != nil {
 		writeError(w, http.StatusBadRequest, "invalid_json", "invalid JSON request body")
 		return
 	}
@@ -152,6 +154,14 @@ func mapRunError(err error) (int, string, string) {
 		return http.StatusNotFound, "prompt_not_found", "prompt definition not found"
 	case errors.Is(err, profile.ErrProfileNotFound):
 		return http.StatusNotFound, "profile_not_found", "execution profile not found"
+	case errors.Is(err, promptdef.ErrInvalidYAML), errors.Is(err, promptdef.ErrInvalidPromptDefinition):
+		return http.StatusBadRequest, "prompt_load_failed", "failed to load prompt definition"
+	case errors.Is(err, profile.ErrInvalidYAML), errors.Is(err, profile.ErrInvalidProfile):
+		return http.StatusBadRequest, "profile_load_failed", "failed to load execution profile"
+	case errors.Is(err, usecase.ErrInvalidRequest) && strings.Contains(err.Error(), "profile id is required either in request or prompt default_profile"):
+		return http.StatusBadRequest, "profile_required", "profile_id is required when prompt default_profile is not set"
+	case errors.Is(err, usecase.ErrInvalidRequest) && strings.Contains(err.Error(), "api key environment variable"):
+		return http.StatusBadRequest, "api_key_env_missing", "api_key_env is set but the environment variable is missing"
 	case errors.Is(err, usecase.ErrInvalidRequest):
 		return http.StatusBadRequest, "invalid_request", "invalid run request"
 	case errors.Is(err, usecase.ErrProfileLoad):

internal/adapter/http/handler_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -13,6 +14,7 @@ import (
 
 	"gitea.maximumdirect.net/eric/scriptorium/internal/domain"
 	"gitea.maximumdirect.net/eric/scriptorium/internal/profile"
+	"gitea.maximumdirect.net/eric/scriptorium/internal/promptdef"
 	"gitea.maximumdirect.net/eric/scriptorium/internal/usecase"
 )
 
@@ -30,9 +32,12 @@ func (f *fakeRunner) Run(ctx context.Context, req domain.RunRequest) (*domain.Ru
 	return f.result, nil
 }
 
-func TestHandlerPostRunsSuccess(t *testing.T) {
+func TestHandlerPostRunsSuccessWithExplicitProfileID(t *testing.T) {
 	start := time.Now().UTC()
 	end := start.Add(2 * time.Second)
+	const envName = "SCRIPTORIUM_API_KEY"
+	const secret = "never-include-me"
+
 	r := &fakeRunner{result: &domain.RunResult{
 		RunID: "11111111-1111-4111-8111-111111111111",
 		Artifact: domain.Artifact{
@@ -57,6 +62,7 @@ func TestHandlerPostRunsSuccess(t *testing.T) {
 			MaxTokens:      42,
 			TopP:           0.9,
 			TimeoutSeconds: 120,
+			APIKeyEnv:      envName,
 		},
 		InputHashes: map[string]string{"transcript": "h1"},
 		Usage:       domain.TokenUsage{PromptTokens: 1, CompletionTokens: 2, TotalTokens: 3},
@@ -75,7 +81,7 @@ func TestHandlerPostRunsSuccess(t *testing.T) {
     "transcript": {"type": "file", "uri": "./t.md"}
   },
   "vars": {"k": "v"},
-  "model": {"model": "gpt-x", "timeout_seconds": 120}
+  "model": {"model": "gpt-x", "timeout_seconds": 120, "api_key_env": "SCRIPTORIUM_API_KEY"}
 }`)
 	req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewReader(body))
 	w := httptest.NewRecorder()
@@ -91,37 +97,25 @@ func TestHandlerPostRunsSuccess(t *testing.T) {
 		t.Fatalf("invalid JSON response: %v", err)
 	}
 
-	artifact := resp["artifact"].(map[string]any)
-	if artifact["body"] != "hello" {
-		t.Fatalf("expected artifact body hello, got %#v", artifact["body"])
-	}
-	validation := resp["validation"].(map[string]any)
-	if validation["status"] != "passed" {
-		t.Fatalf("expected validation.status passed, got %#v", validation["status"])
-	}
 	metadata := resp["metadata"].(map[string]any)
-	if metadata["run_id"] != "11111111-1111-4111-8111-111111111111" {
+	if metadata["prompt_id"] != "prompt-1" {
-		t.Fatalf("unexpected metadata.run_id: %#v", metadata["run_id"])
+		t.Fatalf("unexpected metadata.prompt_id: %#v", metadata["prompt_id"])
 	}
-	if metadata["prompt_hash"] != "phash" {
+	if metadata["prompt_version"] != "1.0.0" {
-		t.Fatalf("unexpected metadata.prompt_hash: %#v", metadata["prompt_hash"])
+		t.Fatalf("unexpected metadata.prompt_version: %#v", metadata["prompt_version"])
 	}
-	usage := metadata["usage"].(map[string]any)
+	if metadata["selected_profile_id"] != "exec-default" {
-	if usage["total_tokens"] != float64(3) {
+		t.Fatalf("unexpected metadata.selected_profile_id: %#v", metadata["selected_profile_id"])
-		t.Fatalf("expected usage.total_tokens=3, got %#v", usage["total_tokens"])
 	}
-	if metadata["duration_ms"] != float64(2000) {
+	if metadata["model_name"] != "m1" || metadata["endpoint"] != "http://llm/v1" {
-		t.Fatalf("expected duration_ms=2000, got %#v", metadata["duration_ms"])
+		t.Fatalf("unexpected model metadata: name=%#v endpoint=%#v", metadata["model_name"], metadata["endpoint"])
-	}
-	if metadata["validation_mode"] != "basic" || metadata["validation_status"] != "passed" {
-		t.Fatalf("unexpected validation metadata: mode=%#v status=%#v", metadata["validation_mode"], metadata["validation_status"])
 	}
 	modelParams := metadata["model_params"].(map[string]any)
-	if modelParams["model"] != "m1" {
+	if modelParams["api_key_env"] != envName {
-		t.Fatalf("unexpected model_params.model: %#v", modelParams["model"])
+		t.Fatalf("expected model_params.api_key_env=%q, got %#v", envName, modelParams["api_key_env"])
 	}
-	if resp["raw_model_output"] != "hello" {
+	if strings.Contains(w.Body.String(), secret) {
-		t.Fatalf("expected raw model output hello, got %#v", resp["raw_model_output"])
+		t.Fatalf("response leaked raw API key value: %s", w.Body.String())
 	}
 
 	if r.last.PromptID != "prompt-1" {
@@ -138,6 +132,37 @@ func TestHandlerPostRunsSuccess(t *testing.T) {
 	}
 }
 
+func TestHandlerPostRunsSuccessUsingPromptDefaultProfile(t *testing.T) {
+	r := &fakeRunner{result: &domain.RunResult{
+		Artifact:             domain.Artifact{Body: []byte("ok")},
+		PromptID:             "prompt-1",
+		PromptVersion:        "1.0.0",
+		SelectedProfileID:    "prompt-default",
+		Validation:           domain.ValidationResult{Status: domain.ValidationPassed, Mode: domain.ValidationBasic, IsValid: true},
+		EffectiveModelParams: domain.ExecutionTarget{Endpoint: "http://llm/v1", Model: "m1"},
+	}}
+	h := NewHandler(r)
+
+	req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString(`{"prompt_id":"prompt-1","inputs":{"x":{"type":"file","uri":"a"}}}`))
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d body=%s", w.Code, w.Body.String())
+	}
+	if r.last.ProfileID != "" {
+		t.Fatalf("expected empty request profile_id when omitted, got %q", r.last.ProfileID)
+	}
+	var resp map[string]any
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("invalid JSON response: %v", err)
+	}
+	metadata := resp["metadata"].(map[string]any)
+	if metadata["selected_profile_id"] != "prompt-default" {
+		t.Fatalf("expected selected_profile_id from result, got %#v", metadata["selected_profile_id"])
+	}
+}
+
 func TestHandlerInvalidJSON(t *testing.T) {
 	h := NewHandler(&fakeRunner{})
 	req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString("{"))
@@ -160,19 +185,35 @@ func TestHandlerMissingPromptID(t *testing.T) {
 	if w.Code != http.StatusBadRequest {
 		t.Fatalf("expected 400, got %d", w.Code)
 	}
+	var resp map[string]any
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("invalid JSON response: %v", err)
+	}
+	errBody := resp["error"].(map[string]any)
+	if errBody["code"] != "invalid_request" {
+		t.Fatalf("expected invalid_request code, got %#v", errBody["code"])
+	}
 }
 
 func TestHandlerUsecaseErrorMapping(t *testing.T) {
 	tests := []struct {
-		name   string
+		name       string
-		err    error
+		err        error
-		status int
+		status     int
+		code       string
+		message    string
+		avoidCause string
 	}{
-		{name: "profile not found", err: wrap(usecase.ErrProfileLoad, profile.ErrProfileNotFound), status: http.StatusNotFound},
+		{name: "prompt not found", err: wrap(usecase.ErrProfileLoad, promptdef.ErrPromptDefinitionNotFound), status: http.StatusNotFound, code: "prompt_not_found", message: "prompt definition not found"},
-		{name: "artifact", err: wrap(usecase.ErrArtifactLoad, fmt.Errorf("read failed")), status: http.StatusBadRequest},
+		{name: "prompt load invalid", err: wrap(usecase.ErrProfileLoad, promptdef.ErrInvalidPromptDefinition), status: http.StatusBadRequest, code: "prompt_load_failed", message: "failed to load prompt definition"},
-		{name: "prompt", err: wrap(usecase.ErrPromptRender, fmt.Errorf("render failed")), status: http.StatusBadRequest},
+		{name: "missing profile/default", err: wrap(usecase.ErrInvalidRequest, errors.New("profile id is required either in request or prompt default_profile")), status: http.StatusBadRequest, code: "profile_required", message: "profile_id is required when prompt default_profile is not set"},
-		{name: "llm", err: wrap(usecase.ErrLLMGenerate, fmt.Errorf("llm failed")), status: http.StatusBadGateway},
+		{name: "profile not found", err: wrap(usecase.ErrProfileLoad, profile.ErrProfileNotFound), status: http.StatusNotFound, code: "profile_not_found", message: "execution profile not found"},
-		{name: "validation runtime", err: wrap(usecase.ErrValidation, fmt.Errorf("validator broke")), status: http.StatusInternalServerError},
+		{name: "profile invalid", err: wrap(usecase.ErrProfileLoad, profile.ErrInvalidProfile), status: http.StatusBadRequest, code: "profile_load_failed", message: "failed to load execution profile"},
+		{name: "api key env missing", err: wrap(usecase.ErrInvalidRequest, errors.New(`api key environment variable "SCRIPTORIUM_API_KEY" is not set`)), status: http.StatusBadRequest, code: "api_key_env_missing", message: "api_key_env is set but the environment variable is missing"},
+		{name: "artifact", err: wrap(usecase.ErrArtifactLoad, fmt.Errorf("read failed")), status: http.StatusBadRequest, code: "artifact_read_failed", message: "failed to read input artifact", avoidCause: "read failed"},
+		{name: "prompt render", err: wrap(usecase.ErrPromptRender, fmt.Errorf("render failed")), status: http.StatusBadRequest, code: "prompt_render_failed", message: "failed to render prompt", avoidCause: "render failed"},
+		{name: "llm", err: wrap(usecase.ErrLLMGenerate, fmt.Errorf("llm failed")), status: http.StatusBadGateway, code: "llm_failed", message: "model generation request failed", avoidCause: "llm failed"},
+		{name: "validation runtime", err: wrap(usecase.ErrValidation, fmt.Errorf("validator broke")), status: http.StatusInternalServerError, code: "validation_runtime_failed", message: "validation runtime failed", avoidCause: "validator broke"},
 	}
 
 	for _, tc := range tests {
@@ -191,19 +232,39 @@ func TestHandlerUsecaseErrorMapping(t *testing.T) {
 				t.Fatalf("invalid JSON response: %v", err)
 			}
 			errBody := resp["error"].(map[string]any)
-			if _, ok := errBody["code"].(string); !ok {
+			if errBody["code"] != tc.code {
-				t.Fatalf("expected error code string, got %#v", errBody["code"])
+				t.Fatalf("expected code %q, got %#v", tc.code, errBody["code"])
 			}
-			if msg, ok := errBody["message"].(string); !ok || msg == "" {
+			if errBody["message"] != tc.message {
-				t.Fatalf("expected non-empty error message, got %#v", errBody["message"])
+				t.Fatalf("expected message %q, got %#v", tc.message, errBody["message"])
 			}
-			if strings.Contains(w.Body.String(), "read failed") || strings.Contains(w.Body.String(), "render failed") || strings.Contains(w.Body.String(), "llm failed") {
+			if tc.avoidCause != "" && strings.Contains(w.Body.String(), tc.avoidCause) {
-				t.Fatalf("expected response to avoid leaking internal cause details, got %s", w.Body.String())
+				t.Fatalf("expected response to avoid leaking cause details, got %s", w.Body.String())
 			}
 		})
 	}
 }
 
+func TestHandlerRawAPIKeyRejectedByStrictJSON(t *testing.T) {
+	h := NewHandler(&fakeRunner{})
+	req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString(`{"prompt_id":"p","inputs":{"x":{"type":"file","uri":"a"}},"model":{"model":"m","api_key":"secret"}}`))
+	w := httptest.NewRecorder()
+
+	h.ServeHTTP(w, req)
+
+	if w.Code != http.StatusBadRequest {
+		t.Fatalf("expected 400, got %d body=%s", w.Code, w.Body.String())
+	}
+	var resp map[string]any
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("invalid JSON response: %v", err)
+	}
+	errBody := resp["error"].(map[string]any)
+	if errBody["code"] != "invalid_json" {
+		t.Fatalf("expected invalid_json code, got %#v", errBody["code"])
+	}
+}
+
 func TestHandlerValidationFailureStillSuccess(t *testing.T) {
 	h := NewHandler(&fakeRunner{result: &domain.RunResult{
 		Artifact:  domain.Artifact{Body: []byte("bad json")},