diff --git a/cmd/generate_changelog/incoming/2206.txt b/cmd/generate_changelog/incoming/2206.txt new file mode 100644 index 00000000..18cf7fab --- /dev/null +++ b/cmd/generate_changelog/incoming/2206.txt @@ -0,0 +1,7 @@ +### PR [#2206](https://github.com/danielmiessler/Fabric/pull/2206) by [ksylvan](https://github.com/ksylvan): fix: confine storage names and authenticate Ollama serve + +- Reject unsafe cross-platform storage names and confine symlink targets to configured storage directories. +- Require API keys for non-loopback server bindings, authenticate Ollama routes, and securely forward configured credentials. +- Sanitize client errors to prevent exposure of internal filesystem paths. +- Validate chat pattern, context, and session names early. +- Default the REST API to loopback-only port 8080 and add regression coverage for traversal, symlink, and authentication vulnerabilities. diff --git a/docs/docs.go b/docs/docs.go index e67df397..8265f8d1 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -139,6 +139,15 @@ const docTemplate = `{ "$ref": "#/definitions/fsdb.Pattern" } }, + "400": { + "description": "Bad Request", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -346,7 +355,6 @@ const docTemplate = `{ "type": "string" }, "language": { - "description": "Add Language field to bind from request", "type": "string" }, "maxTokens": { @@ -412,9 +420,6 @@ const docTemplate = `{ "type": "number", "format": "float64" }, - "updateChan": { - "type": "object" - }, "voice": { "type": "string" } diff --git a/docs/swagger.json b/docs/swagger.json index 749c6e06..90dea7a3 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -133,6 +133,15 @@ "$ref": "#/definitions/fsdb.Pattern" } }, + "400": { + "description": "Bad Request", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -340,7 +349,6 @@ "type": "string" }, "language": { - "description": "Add Language field to bind from request", "type": "string" }, "maxTokens": { @@ -406,9 +414,6 @@ "type": "number", "format": "float64" }, - "updateChan": { - "type": "object" - }, "voice": { "type": "string" } diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 53184584..34a6d87e 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -50,7 +50,6 @@ definitions: imageSize: type: string language: - description: Add Language field to bind from request type: string maxTokens: type: integer @@ -95,8 +94,6 @@ definitions: topP: format: float64 type: number - updateChan: - type: object voice: type: string type: object @@ -265,6 +262,12 @@ paths: description: OK schema: $ref: '#/definitions/fsdb.Pattern' + "400": + description: Bad Request + schema: + additionalProperties: + type: string + type: object "500": description: Internal Server Error schema: diff --git a/internal/cli/flags.go b/internal/cli/flags.go index efd1e06b..683b2fe2 100644 --- a/internal/cli/flags.go +++ b/internal/cli/flags.go @@ -78,8 +78,8 @@ type Flags struct { DryRun bool `long:"dry-run" description:"Show what would be sent to the model without actually sending it"` Serve bool `long:"serve" description:"Serve the Fabric Rest API"` ServeOllama bool `long:"serveOllama" description:"Serve the Fabric Rest API with ollama endpoints"` - ServeAddress string `long:"address" description:"The address to bind the REST API" default:":8080"` - ServeAPIKey string `long:"api-key" description:"API key used to secure server routes" default:""` + ServeAddress string `long:"address" description:"The address to bind the REST API" default:"127.0.0.1:8080"` + ServeAPIKey string `long:"api-key" env:"FABRIC_API_KEY" description:"API key used to secure server routes" default:""` Config string `long:"config" description:"Path to YAML config file"` Version bool `long:"version" description:"Print current version"` ListExtensions bool `long:"listextensions" description:"List all registered extensions"` diff --git a/internal/cli/setup_server.go b/internal/cli/setup_server.go index 03831195..d8feb7f1 100644 --- a/internal/cli/setup_server.go +++ b/internal/cli/setup_server.go @@ -5,6 +5,10 @@ import ( restapi "github.com/danielmiessler/fabric/internal/server" ) +// serveOllama is a seam for tests, because the real entry point blocks +// on a listening socket. +var serveOllama = restapi.ServeOllama + // handleSetupAndServerCommands handles setup and server-related commands // Returns (handled, error) where handled indicates if a command was processed and should exit func handleSetupAndServerCommands(currentFlags *Flags, registry *core.PluginRegistry, version string) (handled bool, err error) { @@ -22,7 +26,7 @@ func handleSetupAndServerCommands(currentFlags *Flags, registry *core.PluginRegi if currentFlags.ServeOllama { registry.ConfigureVendors() - err = restapi.ServeOllama(registry, currentFlags.ServeAddress, version) + err = serveOllama(registry, currentFlags.ServeAddress, version, currentFlags.ServeAPIKey) return true, err } diff --git a/internal/cli/setup_server_test.go b/internal/cli/setup_server_test.go new file mode 100644 index 00000000..7b9229f2 --- /dev/null +++ b/internal/cli/setup_server_test.go @@ -0,0 +1,37 @@ +package cli + +import ( + "testing" + + "github.com/danielmiessler/fabric/internal/core" + "github.com/danielmiessler/fabric/internal/plugins/ai" +) + +// The --serveOllama path must pass the address, version, and API key +// flags through to ServeOllama unchanged. +func TestHandleSetupAndServerCommands_ServeOllamaWiring(t *testing.T) { + var gotAddress, gotVersion, gotKey string + prev := serveOllama + serveOllama = func(_ *core.PluginRegistry, address, version, apiKey string) error { + gotAddress, gotVersion, gotKey = address, version, apiKey + return nil + } + defer func() { serveOllama = prev }() + + registry := &core.PluginRegistry{ + VendorManager: ai.NewVendorsManager(), + VendorsAll: ai.NewVendorsManager(), + } + flags := &Flags{ServeOllama: true, ServeAddress: "127.0.0.1:9999", ServeAPIKey: "secret"} + handled, err := handleSetupAndServerCommands(flags, registry, "v-test") + if err != nil { + t.Fatalf("handleSetupAndServerCommands() error = %v", err) + } + if !handled { + t.Fatal("handleSetupAndServerCommands() handled = false, want true") + } + if gotAddress != "127.0.0.1:9999" || gotVersion != "v-test" || gotKey != "secret" { + t.Fatalf("ServeOllama got (%q, %q, %q), want (127.0.0.1:9999, v-test, secret)", + gotAddress, gotVersion, gotKey) + } +} diff --git a/internal/i18n/locales/de.json b/internal/i18n/locales/de.json index 62e3b4ca..101c2aea 100644 --- a/internal/i18n/locales/de.json +++ b/internal/i18n/locales/de.json @@ -375,6 +375,7 @@ "ollama_invalid_data_url_format": "ungültiges Data-URL-Format", "ollama_invalid_http_timeout_using_default": "ungültiges HTTP-Zeitlimit '%s': %v, verwende Standardwert", "ollama_invalid_num_ctx_in_request": "Ungültiger num_ctx in Anfrage: %v", + "ollama_invalid_request_body": "ungültiger Anfragetext", "ollama_no_content_from_upstream": "Kein Inhalt vom Upstream Fabric Server erhalten", "ollama_num_ctx_exceeds_maximum": "num_ctx überschreitet den maximal zulässigen Wert von %d", "ollama_num_ctx_invalid_type": "num_ctx muss eine Zahl sein, ungültiger Typ erhalten", @@ -388,6 +389,7 @@ "ollama_sse_buffer_limit": "SSE Zeile überschreitet 1MB Puffer-Limit - Datenzeile zu groß", "ollama_upstream_non_2xx": "Upstream Fabric Server hat nicht-2xx Status %d zurückgegeben: %s", "ollama_upstream_non_2xx_body_unreadable": "Upstream Fabric Server hat nicht-2xx Status %d zurückgegeben und Body konnte nicht gelesen werden: %v", + "ollama_upstream_request_failed": "Upstream-Fabric-Server nicht erreichbar", "ollama_upstream_returned_status": "Upstream Fabric Server hat Status %d zurückgegeben", "ollama_warning_no_content": "Warnung: Kein Inhalt vom Upstream Fabric Server erhalten", "ollama_warning_parse_variables": "Warnung: Fehler beim Parsen von options.variables als JSON: %v", @@ -513,10 +515,12 @@ "send_desktop_notification": "Desktop-Benachrichtigung senden, wenn Befehl abgeschlossen ist", "serve_fabric_api_ollama_endpoints": "Fabric REST API mit ollama-Endpunkten bereitstellen", "serve_fabric_rest_api": "Fabric REST API bereitstellen", + "server_api_key_required": "Server-Start auf Nicht-Loopback-Adresse %s ohne API-Schlüssel verweigert: Setzen Sie --api-key oder FABRIC_API_KEY, oder binden Sie eine Loopback-Adresse wie 127.0.0.1:8080", "server_chat_error": "Fehler: %v", "server_error_marshaling_response": "Fehler beim Serialisieren der Antwort: %v", "server_error_writing_response": "Fehler beim Schreiben der Antwort: %v", "server_invalid_request_format": "ungültiges Anfrageformat: %v", + "server_no_api_key_warning": "REST-API-Server wird ohne API-Schlüssel-Authentifizierung gestartet. Dies kann Sicherheitsrisiken bergen.", "sessions_creating_new": "Erstelle neue Sitzung: %s\n", "set_debug_level": "Debug-Level festlegen (0=aus, 1=grundlegend, 2=detailliert, 3=Trace, 4=wire)", "set_frequency_penalty": "Häufigkeitsstrafe festlegen", @@ -620,6 +624,7 @@ "storage_error_save": "%s konnte nicht gespeichert werden: %v", "storage_error_stat_entry": "Eintrag %s konnte nicht abgefragt werden: %v", "storage_error_unmarshal": "%s konnte nicht deserialisiert werden: %s", + "storage_invalid_name": "Ungültiger Name: %q", "strategies_available_header": "Verfügbare Strategien:", "strategies_cloning_repository": "Repository %s wird geklont (Pfad: %s)...\\n", "strategies_download_success": "✅ Strategien erfolgreich nach %s heruntergeladen und installiert\\n", diff --git a/internal/i18n/locales/en.json b/internal/i18n/locales/en.json index 6429521d..660bf9f6 100644 --- a/internal/i18n/locales/en.json +++ b/internal/i18n/locales/en.json @@ -375,6 +375,7 @@ "ollama_invalid_data_url_format": "invalid data URL format", "ollama_invalid_http_timeout_using_default": "invalid HTTP timeout '%s': %v, using default", "ollama_invalid_num_ctx_in_request": "invalid num_ctx in request: %v", + "ollama_invalid_request_body": "invalid request body", "ollama_no_content_from_upstream": "no content received from upstream Fabric server", "ollama_num_ctx_exceeds_maximum": "num_ctx exceeds maximum allowed value of %d", "ollama_num_ctx_invalid_type": "num_ctx must be a number, got invalid type", @@ -388,6 +389,7 @@ "ollama_sse_buffer_limit": "SSE line exceeds 1MB buffer limit - data line too large", "ollama_upstream_non_2xx": "Upstream Fabric server returned non-2xx status %d: %s", "ollama_upstream_non_2xx_body_unreadable": "Upstream Fabric server returned non-2xx status %d and body could not be read: %v", + "ollama_upstream_request_failed": "failed to reach upstream Fabric server", "ollama_upstream_returned_status": "upstream Fabric server returned status %d", "ollama_warning_no_content": "Warning: no content received from upstream Fabric server", "ollama_warning_parse_variables": "Warning: failed to parse options.variables as JSON: %v", @@ -513,10 +515,12 @@ "send_desktop_notification": "Send desktop notification when command completes", "serve_fabric_api_ollama_endpoints": "Serve the Fabric Rest API with ollama endpoints", "serve_fabric_rest_api": "Serve the Fabric Rest API", + "server_api_key_required": "refusing to serve on non-loopback address %s without an API key: set --api-key or FABRIC_API_KEY, or bind a loopback address such as 127.0.0.1:8080", "server_chat_error": "Error: %v", "server_error_marshaling_response": "error marshaling response: %v", "server_error_writing_response": "error writing response: %v", "server_invalid_request_format": "invalid request format: %v", + "server_no_api_key_warning": "Starting REST API server without API key authentication. This may pose security risks.", "sessions_creating_new": "Creating new session: %s\n", "set_debug_level": "Set debug level (0=off, 1=basic, 2=detailed, 3=trace, 4=wire)", "set_frequency_penalty": "Set frequency penalty", @@ -620,6 +624,7 @@ "storage_error_save": "could not save %s: %v", "storage_error_stat_entry": "could not stat entry %s: %v", "storage_error_unmarshal": "could not unmarshal %s: %s", + "storage_invalid_name": "invalid name: %q", "strategies_available_header": "Available Strategies:", "strategies_cloning_repository": "Cloning repository %s (path: %s)...\n", "strategies_download_success": "✅ Successfully downloaded and installed strategies to %s\n", diff --git a/internal/i18n/locales/es.json b/internal/i18n/locales/es.json index 2b544264..bacfb75f 100644 --- a/internal/i18n/locales/es.json +++ b/internal/i18n/locales/es.json @@ -375,6 +375,7 @@ "ollama_invalid_data_url_format": "formato de URL de datos inválido", "ollama_invalid_http_timeout_using_default": "Tiempo de espera HTTP inválido '%s': %v, usando el valor predeterminado", "ollama_invalid_num_ctx_in_request": "num_ctx inválido en la solicitud: %v", + "ollama_invalid_request_body": "cuerpo de solicitud no válido", "ollama_no_content_from_upstream": "no se recibió contenido del servidor Fabric upstream", "ollama_num_ctx_exceeds_maximum": "num_ctx excede el valor máximo permitido de %d", "ollama_num_ctx_invalid_type": "num_ctx debe ser un número, se obtuvo tipo inválido", @@ -388,6 +389,7 @@ "ollama_sse_buffer_limit": "Línea SSE excede el límite de búfer de 1MB - línea de datos demasiado grande", "ollama_upstream_non_2xx": "El servidor Fabric upstream devolvió estado no-2xx %d: %s", "ollama_upstream_non_2xx_body_unreadable": "El servidor Fabric upstream devolvió estado no-2xx %d y el cuerpo no se pudo leer: %v", + "ollama_upstream_request_failed": "no se pudo conectar con el servidor Fabric ascendente", "ollama_upstream_returned_status": "el servidor Fabric upstream devolvió estado %d", "ollama_warning_no_content": "Advertencia: no se recibió contenido del servidor Fabric upstream", "ollama_warning_parse_variables": "Advertencia: error al analizar options.variables como JSON: %v", @@ -513,10 +515,12 @@ "send_desktop_notification": "Enviar notificación de escritorio cuando se complete el comando", "serve_fabric_api_ollama_endpoints": "Servir la API REST de Fabric con endpoints de ollama", "serve_fabric_rest_api": "Servir la API REST de Fabric", + "server_api_key_required": "se rechaza servir en la dirección no loopback %s sin clave de API: configure --api-key o FABRIC_API_KEY, o use una dirección loopback como 127.0.0.1:8080", "server_chat_error": "Error: %v", "server_error_marshaling_response": "error al serializar la respuesta: %v", "server_error_writing_response": "error al escribir la respuesta: %v", "server_invalid_request_format": "formato de solicitud no válido: %v", + "server_no_api_key_warning": "Iniciando el servidor de API REST sin autenticación por clave de API. Esto puede suponer riesgos de seguridad.", "sessions_creating_new": "Creando nueva sesión: %s\n", "set_debug_level": "Establecer nivel de depuración (0=apagado, 1=básico, 2=detallado, 3=rastreo, 4=wire)", "set_frequency_penalty": "Establecer penalización de frecuencia", @@ -620,6 +624,7 @@ "storage_error_save": "No se pudo guardar %s: %v", "storage_error_stat_entry": "No se pudo obtener información de la entrada %s: %v", "storage_error_unmarshal": "No se pudo deserializar %s: %s", + "storage_invalid_name": "nombre inválido: %q", "strategies_available_header": "Estrategias disponibles:", "strategies_cloning_repository": "Clonando el repositorio %s (ruta: %s)...\\n", "strategies_download_success": "✅ Estrategias descargadas e instaladas correctamente en %s\\n", diff --git a/internal/i18n/locales/fa.json b/internal/i18n/locales/fa.json index 7bb0eb8f..b9a632f4 100644 --- a/internal/i18n/locales/fa.json +++ b/internal/i18n/locales/fa.json @@ -375,6 +375,7 @@ "ollama_invalid_data_url_format": "فرمت URL داده نامعتبر", "ollama_invalid_http_timeout_using_default": "زمان انتظار HTTP نامعتبر '%s': %v، استفاده از مقدار پیش‌فرض", "ollama_invalid_num_ctx_in_request": "num_ctx نامعتبر در درخواست: %v", + "ollama_invalid_request_body": "بدنه درخواست نامعتبر", "ollama_no_content_from_upstream": "هیچ محتوایی از سرور Fabric بالادستی دریافت نشد", "ollama_num_ctx_exceeds_maximum": "num_ctx از حداکثر مقدار مجاز %d فراتر رفته است", "ollama_num_ctx_invalid_type": "num_ctx باید یک عدد باشد، نوع نامعتبر دریافت شد", @@ -388,6 +389,7 @@ "ollama_sse_buffer_limit": "خط SSE از حد بافر 1MB فراتر رفت - خط داده بیش از حد بزرگ است", "ollama_upstream_non_2xx": "سرور Fabric بالادستی وضعیت غیر-2xx %d را برگرداند: %s", "ollama_upstream_non_2xx_body_unreadable": "سرور Fabric بالادستی وضعیت غیر-2xx %d را برگرداند و بدنه قابل خواندن نبود: %v", + "ollama_upstream_request_failed": "دسترسی به سرور بالادستی Fabric ممکن نیست", "ollama_upstream_returned_status": "سرور Fabric بالادستی وضعیت %d را برگرداند", "ollama_warning_no_content": "هشدار: هیچ محتوایی از سرور Fabric بالادستی دریافت نشد", "ollama_warning_parse_variables": "هشدار: شکست در تجزیه options.variables به عنوان JSON: %v", @@ -513,10 +515,12 @@ "send_desktop_notification": "ارسال اعلان دسک‌تاپ هنگام تکمیل دستور", "serve_fabric_api_ollama_endpoints": "سرویس API REST Fabric با نقاط پایانی ollama", "serve_fabric_rest_api": "سرویس API REST Fabric", + "server_api_key_required": "سرویس‌دهی روی آدرس غیر loopback ‏%s بدون کلید API رد شد: ‏--api-key یا FABRIC_API_KEY را تنظیم کنید، یا به یک آدرس loopback مانند 127.0.0.1:8080 متصل شوید", "server_chat_error": "خطا: %v", "server_error_marshaling_response": "خطا در سریال‌سازی پاسخ: %v", "server_error_writing_response": "خطا در نوشتن پاسخ: %v", "server_invalid_request_format": "فرمت درخواست نامعتبر: %v", + "server_no_api_key_warning": "سرور REST API بدون احراز هویت کلید API راه‌اندازی می‌شود. این ممکن است خطرات امنیتی ایجاد کند.", "sessions_creating_new": "ایجاد نشست جدید: %s\n", "set_debug_level": "تنظیم سطح اشکال‌زدایی (0=خاموش، 1=پایه، 2=تفصیلی، 3=ردیابی، 4=wire)", "set_frequency_penalty": "تنظیم جریمه فرکانس", @@ -620,6 +624,7 @@ "storage_error_save": "ذخیره %s ناموفق بود: %v", "storage_error_stat_entry": "دریافت اطلاعات ورودی %s ناموفق بود: %v", "storage_error_unmarshal": "بازسریال‌سازی %s ناموفق بود: %s", + "storage_invalid_name": "نام نامعتبر: %q", "strategies_available_header": "راهبردهای موجود:", "strategies_cloning_repository": "در حال کلون کردن مخزن %s (مسیر: %s)...\\n", "strategies_download_success": "✅ راهبردها با موفقیت در %s دانلود و نصب شدند\\n", diff --git a/internal/i18n/locales/fr.json b/internal/i18n/locales/fr.json index d8b4f7f3..a32af818 100644 --- a/internal/i18n/locales/fr.json +++ b/internal/i18n/locales/fr.json @@ -375,6 +375,7 @@ "ollama_invalid_data_url_format": "format d'URL de données invalide", "ollama_invalid_http_timeout_using_default": "Délai d'expiration HTTP invalide '%s' : %v, utilisation de la valeur par défaut", "ollama_invalid_num_ctx_in_request": "num_ctx invalide dans la requête : %v", + "ollama_invalid_request_body": "corps de requête invalide", "ollama_no_content_from_upstream": "aucun contenu reçu du serveur Fabric en amont", "ollama_num_ctx_exceeds_maximum": "num_ctx dépasse la valeur maximale autorisée de %d", "ollama_num_ctx_invalid_type": "num_ctx doit être un nombre, type invalide reçu", @@ -388,6 +389,7 @@ "ollama_sse_buffer_limit": "La ligne SSE dépasse la limite de tampon de 1 Mo - ligne de données trop grande", "ollama_upstream_non_2xx": "Le serveur Fabric en amont a renvoyé un statut non-2xx %d : %s", "ollama_upstream_non_2xx_body_unreadable": "Le serveur Fabric en amont a renvoyé un statut non-2xx %d et le corps n'a pas pu être lu : %v", + "ollama_upstream_request_failed": "impossible de joindre le serveur Fabric en amont", "ollama_upstream_returned_status": "le serveur Fabric en amont a renvoyé le statut %d", "ollama_warning_no_content": "Attention : aucun contenu reçu du serveur Fabric en amont", "ollama_warning_parse_variables": "Attention : échec de l'analyse de options.variables en JSON : %v", @@ -513,10 +515,12 @@ "send_desktop_notification": "Envoyer une notification de bureau quand la commande se termine", "serve_fabric_api_ollama_endpoints": "Servir l'API REST Fabric avec les endpoints ollama", "serve_fabric_rest_api": "Servir l'API REST Fabric", + "server_api_key_required": "refus de servir sur l'adresse non loopback %s sans clé API : définissez --api-key ou FABRIC_API_KEY, ou liez une adresse loopback comme 127.0.0.1:8080", "server_chat_error": "Erreur : %v", "server_error_marshaling_response": "erreur de sérialisation de la réponse : %v", "server_error_writing_response": "erreur d'écriture de la réponse : %v", "server_invalid_request_format": "format de requête invalide : %v", + "server_no_api_key_warning": "Démarrage du serveur API REST sans authentification par clé API. Cela peut présenter des risques de sécurité.", "sessions_creating_new": "Création d'une nouvelle session : %s\n", "set_debug_level": "Définir le niveau de débogage (0=désactivé, 1=basique, 2=détaillé, 3=trace, 4=wire)", "set_frequency_penalty": "Définir la pénalité de fréquence", @@ -620,6 +624,7 @@ "storage_error_save": "Impossible de sauvegarder %s : %v", "storage_error_stat_entry": "Impossible d'obtenir les informations de l'entrée %s : %v", "storage_error_unmarshal": "Impossible de désérialiser %s : %s", + "storage_invalid_name": "nom invalide : %q", "strategies_available_header": "Stratégies disponibles :", "strategies_cloning_repository": "Clonage du dépôt %s (chemin : %s)...\\n", "strategies_download_success": "✅ Stratégies téléchargées et installées avec succès dans %s\\n", diff --git a/internal/i18n/locales/it.json b/internal/i18n/locales/it.json index 99be76a5..6e261509 100644 --- a/internal/i18n/locales/it.json +++ b/internal/i18n/locales/it.json @@ -375,6 +375,7 @@ "ollama_invalid_data_url_format": "formato URL dati non valido", "ollama_invalid_http_timeout_using_default": "Timeout HTTP non valido '%s': %v, utilizzo del valore predefinito", "ollama_invalid_num_ctx_in_request": "num_ctx non valido nella richiesta: %v", + "ollama_invalid_request_body": "corpo della richiesta non valido", "ollama_no_content_from_upstream": "nessun contenuto ricevuto dal server Fabric upstream", "ollama_num_ctx_exceeds_maximum": "num_ctx supera il valore massimo consentito di %d", "ollama_num_ctx_invalid_type": "num_ctx deve essere un numero, ricevuto tipo non valido", @@ -388,6 +389,7 @@ "ollama_sse_buffer_limit": "La riga SSE supera il limite del buffer di 1MB - riga di dati troppo grande", "ollama_upstream_non_2xx": "Il server Fabric upstream ha restituito stato non-2xx %d: %s", "ollama_upstream_non_2xx_body_unreadable": "Il server Fabric upstream ha restituito stato non-2xx %d e il corpo non è stato leggibile: %v", + "ollama_upstream_request_failed": "impossibile raggiungere il server Fabric a monte", "ollama_upstream_returned_status": "il server Fabric upstream ha restituito stato %d", "ollama_warning_no_content": "Avviso: nessun contenuto ricevuto dal server Fabric upstream", "ollama_warning_parse_variables": "Avviso: impossibile analizzare options.variables come JSON: %v", @@ -513,10 +515,12 @@ "send_desktop_notification": "Invia notifica desktop quando il comando è completato", "serve_fabric_api_ollama_endpoints": "Servi l'API REST di Fabric con endpoint ollama", "serve_fabric_rest_api": "Servi l'API REST di Fabric", + "server_api_key_required": "rifiuto di servire sull'indirizzo non loopback %s senza chiave API: impostare --api-key o FABRIC_API_KEY, oppure associare un indirizzo loopback come 127.0.0.1:8080", "server_chat_error": "Errore: %v", "server_error_marshaling_response": "errore nella serializzazione della risposta: %v", "server_error_writing_response": "errore nella scrittura della risposta: %v", "server_invalid_request_format": "formato della richiesta non valido: %v", + "server_no_api_key_warning": "Avvio del server API REST senza autenticazione con chiave API. Ciò può comportare rischi per la sicurezza.", "sessions_creating_new": "Creazione nuova sessione: %s\n", "set_debug_level": "Imposta livello di debug (0=spento, 1=base, 2=dettagliato, 3=traccia, 4=wire)", "set_frequency_penalty": "Imposta penalità di frequenza", @@ -620,6 +624,7 @@ "storage_error_save": "Impossibile salvare %s: %v", "storage_error_stat_entry": "Impossibile ottenere informazioni sulla voce %s: %v", "storage_error_unmarshal": "Impossibile deserializzare %s: %s", + "storage_invalid_name": "nome non valido: %q", "strategies_available_header": "Strategie disponibili:", "strategies_cloning_repository": "Clonazione del repository %s (percorso: %s)...\\n", "strategies_download_success": "✅ Strategie scaricate e installate correttamente in %s\\n", diff --git a/internal/i18n/locales/ja.json b/internal/i18n/locales/ja.json index 8c15a701..5041c097 100644 --- a/internal/i18n/locales/ja.json +++ b/internal/i18n/locales/ja.json @@ -375,6 +375,7 @@ "ollama_invalid_data_url_format": "無効なデータURLフォーマット", "ollama_invalid_http_timeout_using_default": "無効なHTTPタイムアウト '%s': %v、デフォルトを使用します", "ollama_invalid_num_ctx_in_request": "リクエストに無効な num_ctx があります: %v", + "ollama_invalid_request_body": "無効なリクエスト本文", "ollama_no_content_from_upstream": "アップストリームの Fabric サーバーからコンテンツを受信しませんでした", "ollama_num_ctx_exceeds_maximum": "num_ctx が許可される最大値 %d を超えています", "ollama_num_ctx_invalid_type": "num_ctx は数値である必要があります。無効な型を受け取りました", @@ -388,6 +389,7 @@ "ollama_sse_buffer_limit": "SSE 行が 1MB バッファー制限を超えています - データ行が大きすぎます", "ollama_upstream_non_2xx": "アップストリームの Fabric サーバーが非 2xx ステータス %d を返しました: %s", "ollama_upstream_non_2xx_body_unreadable": "アップストリームの Fabric サーバーが非 2xx ステータス %d を返し、ボディを読み取れませんでした: %v", + "ollama_upstream_request_failed": "上流のFabricサーバーに到達できませんでした", "ollama_upstream_returned_status": "アップストリームの Fabric サーバーがステータス %d を返しました", "ollama_warning_no_content": "警告: アップストリームの Fabric サーバーからコンテンツを受信しませんでした", "ollama_warning_parse_variables": "警告: options.variables を JSON として解析できませんでした: %v", @@ -513,10 +515,12 @@ "send_desktop_notification": "コマンド完了時にデスクトップ通知を送信", "serve_fabric_api_ollama_endpoints": "ollamaエンドポイント付きのFabric REST APIを提供", "serve_fabric_rest_api": "Fabric REST APIを提供", + "server_api_key_required": "APIキーなしで非ループバックアドレス %s での提供を拒否しました。--api-key または FABRIC_API_KEY を設定するか、127.0.0.1:8080 のようなループバックアドレスにバインドしてください", "server_chat_error": "エラー: %v", "server_error_marshaling_response": "レスポンスのシリアライズエラー: %v", "server_error_writing_response": "レスポンスの書き込みエラー: %v", "server_invalid_request_format": "無効なリクエスト形式: %v", + "server_no_api_key_warning": "APIキー認証なしでREST APIサーバーを起動しています。セキュリティ上のリスクが生じる可能性があります。", "sessions_creating_new": "新しいセッションを作成中: %s\n", "set_debug_level": "デバッグレベルを設定（0=オフ、1=基本、2=詳細、3=トレース、4=wire）", "set_frequency_penalty": "頻度ペナルティを設定", @@ -620,6 +624,7 @@ "storage_error_save": "%sを保存できませんでした: %v", "storage_error_stat_entry": "エントリ%sの情報を取得できませんでした: %v", "storage_error_unmarshal": "%sをデシリアライズできませんでした: %s", + "storage_invalid_name": "無効な名前: %q", "strategies_available_header": "利用可能な戦略:", "strategies_cloning_repository": "リポジトリ %s をクローン中 (パス: %s)...\\n", "strategies_download_success": "✅ 戦略を %s に正常にダウンロードしてインストールしました\\n", diff --git a/internal/i18n/locales/pl.json b/internal/i18n/locales/pl.json index 09f99ca9..84fa54b9 100644 --- a/internal/i18n/locales/pl.json +++ b/internal/i18n/locales/pl.json @@ -375,6 +375,7 @@ "ollama_invalid_data_url_format": "nieprawidłowy format data URL", "ollama_invalid_http_timeout_using_default": "nieprawidłowy limit czasu HTTP '%s': %v, używam domyślnego", "ollama_invalid_num_ctx_in_request": "nieprawidłowa wartość num_ctx w żądaniu: %v", + "ollama_invalid_request_body": "nieprawidłowa treść żądania", "ollama_no_content_from_upstream": "nie odebrano zawartości z upstream serwera fabric", "ollama_num_ctx_exceeds_maximum": "num_ctx przekracza maksymalną dozwoloną wartość %d", "ollama_num_ctx_invalid_type": "num_ctx musi być liczbą, podano nieprawidłowy typ", @@ -388,6 +389,7 @@ "ollama_sse_buffer_limit": "Linia SSE przekracza limit bufora 1MB - linia danych zbyt duża", "ollama_upstream_non_2xx": "Upstream serwer fabric zwrócił status inny niż 2xx %d: %s", "ollama_upstream_non_2xx_body_unreadable": "Upstream serwer fabric zwrócił status inny niż 2xx %d i nie można odczytać treści: %v", + "ollama_upstream_request_failed": "nie można połączyć się z serwerem nadrzędnym Fabric", "ollama_upstream_returned_status": "upstream serwer fabric zwrócił status %d", "ollama_warning_no_content": "Ostrzeżenie: nie odebrano zawartości z upstream serwera fabric", "ollama_warning_parse_variables": "Ostrzeżenie: nie udało się przetworzyć options.variables jako JSON: %v", @@ -513,10 +515,12 @@ "send_desktop_notification": "Wyślij powiadomienie pulpitu po zakończeniu polecenia", "serve_fabric_api_ollama_endpoints": "Uruchom fabric Rest API z endpointami ollama", "serve_fabric_rest_api": "Uruchom fabric Rest API", + "server_api_key_required": "odmowa udostępniania na adresie %s spoza pętli zwrotnej bez klucza API: ustaw --api-key lub FABRIC_API_KEY, albo powiąż adres pętli zwrotnej, np. 127.0.0.1:8080", "server_chat_error": "Błąd: %v", "server_error_marshaling_response": "błąd podczas serializacji odpowiedzi: %v", "server_error_writing_response": "błąd podczas zapisywania odpowiedzi: %v", "server_invalid_request_format": "nieprawidłowy format żądania: %v", + "server_no_api_key_warning": "Uruchamianie serwera REST API bez uwierzytelniania kluczem API. Może to stwarzać zagrożenia bezpieczeństwa.", "sessions_creating_new": "Tworzenie nowej sesji: %s\n", "set_debug_level": "Ustaw poziom debugowania (0=wyłączone, 1=podstawowe, 2=szczegółowe, 3=śledzenie, 4=surowe)", "set_frequency_penalty": "Ustaw karę częstotliwości", @@ -620,6 +624,7 @@ "storage_error_save": "nie można zapisać %s: %v", "storage_error_stat_entry": "nie można pobrać informacji o wpisie %s: %v", "storage_error_unmarshal": "nie można deserializować %s: %s", + "storage_invalid_name": "nieprawidłowa nazwa: %q", "strategies_available_header": "Dostępne strategie:", "strategies_cloning_repository": "Klonowanie repozytorium %s (ścieżka: %s)...\n", "strategies_download_success": "✅ Pomyślnie pobrano i zainstalowano strategie w %s\n", diff --git a/internal/i18n/locales/pt-BR.json b/internal/i18n/locales/pt-BR.json index 5e308318..8b349cd2 100644 --- a/internal/i18n/locales/pt-BR.json +++ b/internal/i18n/locales/pt-BR.json @@ -375,6 +375,7 @@ "ollama_invalid_data_url_format": "formato de URL de dados inválido", "ollama_invalid_http_timeout_using_default": "Tempo limite HTTP inválido '%s': %v, usando o padrão", "ollama_invalid_num_ctx_in_request": "num_ctx inválido na requisição: %v", + "ollama_invalid_request_body": "corpo de solicitação inválido", "ollama_no_content_from_upstream": "nenhum conteúdo recebido do servidor Fabric upstream", "ollama_num_ctx_exceeds_maximum": "num_ctx excede o valor máximo permitido de %d", "ollama_num_ctx_invalid_type": "num_ctx deve ser um número, recebeu tipo inválido", @@ -388,6 +389,7 @@ "ollama_sse_buffer_limit": "Linha SSE excede limite de buffer de 1MB - linha de dados muito grande", "ollama_upstream_non_2xx": "Servidor Fabric upstream retornou status não-2xx %d: %s", "ollama_upstream_non_2xx_body_unreadable": "Servidor Fabric upstream retornou status não-2xx %d e o corpo não pôde ser lido: %v", + "ollama_upstream_request_failed": "falha ao conectar ao servidor Fabric upstream", "ollama_upstream_returned_status": "servidor Fabric upstream retornou status %d", "ollama_warning_no_content": "Aviso: nenhum conteúdo recebido do servidor Fabric upstream", "ollama_warning_parse_variables": "Aviso: falha ao analisar options.variables como JSON: %v", @@ -513,10 +515,12 @@ "send_desktop_notification": "Enviar notificação desktop quando o comando for concluído", "serve_fabric_api_ollama_endpoints": "Servir a API REST do Fabric com endpoints ollama", "serve_fabric_rest_api": "Servir a API REST do Fabric", + "server_api_key_required": "recusando servir no endereço não loopback %s sem chave de API: defina --api-key ou FABRIC_API_KEY, ou vincule um endereço loopback como 127.0.0.1:8080", "server_chat_error": "Erro: %v", "server_error_marshaling_response": "erro ao serializar resposta: %v", "server_error_writing_response": "erro ao escrever resposta: %v", "server_invalid_request_format": "formato de solicitação inválido: %v", + "server_no_api_key_warning": "Iniciando o servidor da API REST sem autenticação por chave de API. Isso pode representar riscos de segurança.", "sessions_creating_new": "Criando nova sessão: %s\n", "set_debug_level": "Definir nível de debug (0=desligado, 1=básico, 2=detalhado, 3=rastreamento, 4=wire)", "set_frequency_penalty": "Definir penalidade de frequência", @@ -620,6 +624,7 @@ "storage_error_save": "Não foi possível salvar %s: %v", "storage_error_stat_entry": "Não foi possível obter informações da entrada %s: %v", "storage_error_unmarshal": "Não foi possível desserializar %s: %s", + "storage_invalid_name": "nome inválido: %q", "strategies_available_header": "Estratégias disponíveis:", "strategies_cloning_repository": "Clonando repositório %s (caminho: %s)...\\n", "strategies_download_success": "✅ Estratégias baixadas e instaladas com sucesso em %s\\n", diff --git a/internal/i18n/locales/pt-PT.json b/internal/i18n/locales/pt-PT.json index a8587499..c5fd441b 100644 --- a/internal/i18n/locales/pt-PT.json +++ b/internal/i18n/locales/pt-PT.json @@ -375,6 +375,7 @@ "ollama_invalid_data_url_format": "formato de URL de dados inválido", "ollama_invalid_http_timeout_using_default": "Tempo limite HTTP inválido '%s': %v, a utilizar o padrão", "ollama_invalid_num_ctx_in_request": "num_ctx inválido no pedido: %v", + "ollama_invalid_request_body": "corpo de pedido inválido", "ollama_no_content_from_upstream": "nenhum conteúdo recebido do servidor Fabric upstream", "ollama_num_ctx_exceeds_maximum": "num_ctx excede o valor máximo permitido de %d", "ollama_num_ctx_invalid_type": "num_ctx deve ser um número, recebeu tipo inválido", @@ -388,6 +389,7 @@ "ollama_sse_buffer_limit": "Linha SSE excede limite de buffer de 1MB - linha de dados demasiado grande", "ollama_upstream_non_2xx": "Servidor Fabric upstream retornou estado não-2xx %d: %s", "ollama_upstream_non_2xx_body_unreadable": "Servidor Fabric upstream retornou estado não-2xx %d e o corpo não pôde ser lido: %v", + "ollama_upstream_request_failed": "falha ao contactar o servidor Fabric a montante", "ollama_upstream_returned_status": "servidor Fabric upstream retornou estado %d", "ollama_warning_no_content": "Aviso: nenhum conteúdo recebido do servidor Fabric upstream", "ollama_warning_parse_variables": "Aviso: falha ao analisar options.variables como JSON: %v", @@ -513,10 +515,12 @@ "send_desktop_notification": "Enviar notificação no ambiente de trabalho quando o comando for concluído", "serve_fabric_api_ollama_endpoints": "Servir a API REST do Fabric com endpoints ollama", "serve_fabric_rest_api": "Servir a API REST do Fabric", + "server_api_key_required": "recusa de servir no endereço não loopback %s sem chave de API: defina --api-key ou FABRIC_API_KEY, ou vincule um endereço loopback como 127.0.0.1:8080", "server_chat_error": "Erro: %v", "server_error_marshaling_response": "erro ao serializar resposta: %v", "server_error_writing_response": "erro ao escrever resposta: %v", "server_invalid_request_format": "formato de pedido inválido: %v", + "server_no_api_key_warning": "A iniciar o servidor da API REST sem autenticação por chave de API. Isto pode representar riscos de segurança.", "sessions_creating_new": "A criar nova sessão: %s\n", "set_debug_level": "Definir nível de debug (0=desligado, 1=básico, 2=detalhado, 3=rastreio, 4=wire)", "set_frequency_penalty": "Definir penalidade de frequência", @@ -620,6 +624,7 @@ "storage_error_save": "Não foi possível guardar %s: %v", "storage_error_stat_entry": "Não foi possível obter informações da entrada %s: %v", "storage_error_unmarshal": "Não foi possível desserializar %s: %s", + "storage_invalid_name": "nome inválido: %q", "strategies_available_header": "Estratégias disponíveis:", "strategies_cloning_repository": "A clonar repositório %s (caminho: %s)...\\n", "strategies_download_success": "✅ Estratégias transferidas e instaladas com sucesso em %s\\n", diff --git a/internal/i18n/locales/zh.json b/internal/i18n/locales/zh.json index 2f812ae8..78837807 100644 --- a/internal/i18n/locales/zh.json +++ b/internal/i18n/locales/zh.json @@ -375,6 +375,7 @@ "ollama_invalid_data_url_format": "无效的数据 URL 格式", "ollama_invalid_http_timeout_using_default": "无效的 HTTP 超时时间 '%s'：%v，使用默认值", "ollama_invalid_num_ctx_in_request": "请求中的 num_ctx 无效：%v", + "ollama_invalid_request_body": "无效的请求正文", "ollama_no_content_from_upstream": "未从上游 Fabric 服务器收到内容", "ollama_num_ctx_exceeds_maximum": "num_ctx 超过允许的最大值 %d", "ollama_num_ctx_invalid_type": "num_ctx 必须是数字，收到无效类型", @@ -388,6 +389,7 @@ "ollama_sse_buffer_limit": "SSE 行超过 1MB 缓冲区限制 - 数据行过大", "ollama_upstream_non_2xx": "上游 Fabric 服务器返回非 2xx 状态 %d：%s", "ollama_upstream_non_2xx_body_unreadable": "上游 Fabric 服务器返回非 2xx 状态 %d 且无法读取正文：%v", + "ollama_upstream_request_failed": "无法连接上游 Fabric 服务器", "ollama_upstream_returned_status": "上游 Fabric 服务器返回状态 %d", "ollama_warning_no_content": "警告：未从上游 Fabric 服务器收到内容", "ollama_warning_parse_variables": "警告：无法将 options.variables 解析为 JSON：%v", @@ -513,10 +515,12 @@ "send_desktop_notification": "命令完成时发送桌面通知", "serve_fabric_api_ollama_endpoints": "提供带有 ollama 端点的 Fabric REST API 服务", "serve_fabric_rest_api": "提供 Fabric REST API 服务", + "server_api_key_required": "拒绝在非回环地址 %s 上提供服务（未配置 API 密钥）：请设置 --api-key 或 FABRIC_API_KEY，或绑定回环地址（如 127.0.0.1:8080）", "server_chat_error": "错误：%v", "server_error_marshaling_response": "序列化响应错误：%v", "server_error_writing_response": "写入响应错误：%v", "server_invalid_request_format": "无效的请求格式：%v", + "server_no_api_key_warning": "正在启动 REST API 服务器，未启用 API 密钥身份验证。这可能带来安全风险。", "sessions_creating_new": "正在创建新会话：%s\n", "set_debug_level": "设置调试级别（0=关闭，1=基本，2=详细，3=跟踪，4=wire）", "set_frequency_penalty": "设置频率惩罚", @@ -620,6 +624,7 @@ "storage_error_save": "无法保存 %s：%v", "storage_error_stat_entry": "无法获取条目 %s 的信息：%v", "storage_error_unmarshal": "无法反序列化 %s：%s", + "storage_invalid_name": "无效的名称：%q", "strategies_available_header": "可用的策略：", "strategies_cloning_repository": "正在克隆仓库 %s（至路径：%s）...\\n", "strategies_download_success": "✅ 已成功下载并安装策略到 %s\\n", diff --git a/internal/plugins/db/api.go b/internal/plugins/db/api.go index bb2c3014..b7a605b0 100644 --- a/internal/plugins/db/api.go +++ b/internal/plugins/db/api.go @@ -1,10 +1,15 @@ package db +// Storage is the contract for a named-entity store. Each implementation +// specifies the names that are valid. It rejects an invalid name with a +// typed error. Callers can map this error to a client error. type Storage[T any] interface { Configure() (err error) Get(name string) (ret *T, err error) GetNames() (ret []string, err error) Delete(name string) (err error) + // Exists reports false for an invalid name. It cannot show the + // difference between a rejected name and an absent entry. Exists(name string) (ret bool) Rename(oldName, newName string) (err error) Save(name string, content []byte) (err error) diff --git a/internal/plugins/db/fsdb/patterns.go b/internal/plugins/db/fsdb/patterns.go index e58fff75..54760acf 100644 --- a/internal/plugins/db/fsdb/patterns.go +++ b/internal/plugins/db/fsdb/patterns.go @@ -55,14 +55,18 @@ func (o *PatternsEntity) GetRaw(name string) (*Pattern, error) { return o.getFromDB(name) } -func (o *PatternsEntity) loadPattern(source string) (pattern *Pattern, err error) { - // Determine if this is a file path - isFilePath := strings.HasPrefix(source, "\\") || +// LooksLikePatternFilePath reports whether loadPattern uses source as a +// filesystem path. HTTP handlers must reject these names to keep the +// CLI file-path feature out of the REST API. +func LooksLikePatternFilePath(source string) bool { + return strings.HasPrefix(source, "\\") || strings.HasPrefix(source, "/") || strings.HasPrefix(source, "~") || strings.HasPrefix(source, ".") +} - if isFilePath { +func (o *PatternsEntity) loadPattern(source string) (pattern *Pattern, err error) { + if LooksLikePatternFilePath(source) { // Resolve the file path using GetAbsolutePath var absPath string if absPath, err = util.GetAbsolutePath(source); err != nil { @@ -119,8 +123,13 @@ func (o *PatternsEntity) applyVariables( // retrieves a pattern from the database by name func (o *PatternsEntity) getFromDB(name string) (ret *Pattern, err error) { - if strings.Contains(name, "..") { - return nil, fmt.Errorf(i18n.T("pattern_invalid_name"), name) + if ValidateStorageName(name) != nil { + // The typed error lets an HTTP route without a pre-validation + // guard map this rejection to 400, not 500. + return nil, &InvalidStorageNameError{ + Name: name, + Message: fmt.Sprintf(i18n.T("pattern_invalid_name"), name), + } } // First check custom patterns directory if it exists @@ -283,13 +292,45 @@ func (o *PatternsEntity) Get(name string) (*Pattern, error) { return o.GetApplyVariables(name, nil, "") } func (o *PatternsEntity) Save(name string, content []byte) (err error) { - patternDir := filepath.Join(o.Dir, name) + // Do not store a name that loadPattern uses as a file path, for + // example ".foo" or "~bar". For such a name, GetApplyVariables reads + // from the filesystem, not from the database. + if LooksLikePatternFilePath(name) { + return &InvalidStorageNameError{ + Name: name, + Message: fmt.Sprintf(i18n.T("pattern_invalid_name"), name), + } + } + var patternDir string + if patternDir, err = o.resolvedPath(name); err != nil { + return + } if err = os.MkdirAll(patternDir, os.ModePerm); err != nil { return fmt.Errorf(i18n.T("patterns_error_create_directory"), err) } patternPath := filepath.Join(patternDir, o.SystemPatternFile) + // The pattern file can be a symlink that already exists. Do not + // write through a symlink that goes out of the pattern directory. + if err = symlinkContained(patternDir, patternPath, name); err != nil { + return err + } if err = os.WriteFile(patternPath, content, 0644); err != nil { return fmt.Errorf(i18n.T("patterns_error_save_pattern"), err) } return nil } + +// Rename applies the file-path guard from Save to the destination name. +// Without the guard, the inherited StorageEntity.Rename accepts ".foo" +// or "~foo", and loadPattern then reads these names from the +// filesystem. A path-like source stays permitted, which lets you rename +// a legacy entry to a valid name. +func (o *PatternsEntity) Rename(oldName, newName string) error { + if LooksLikePatternFilePath(newName) { + return &InvalidStorageNameError{ + Name: newName, + Message: fmt.Sprintf(i18n.T("pattern_invalid_name"), newName), + } + } + return o.StorageEntity.Rename(oldName, newName) +} diff --git a/internal/plugins/db/fsdb/patterns_test.go b/internal/plugins/db/fsdb/patterns_test.go index 56d6363e..2ce3b39e 100644 --- a/internal/plugins/db/fsdb/patterns_test.go +++ b/internal/plugins/db/fsdb/patterns_test.go @@ -3,8 +3,10 @@ package fsdb import ( "os" "path/filepath" + "strings" "testing" + "github.com/danielmiessler/fabric/internal/i18n" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -179,6 +181,88 @@ func TestPatternsEntity_Save(t *testing.T) { assert.Equal(t, content, data) } +// Save must reject a name that loadPattern uses as a filesystem path, +// and that includes a traversal name. For such a name, +// GetApplyVariables reads from the disk, not from the database. +func TestPatternsEntity_SaveRejectsFilePathNames(t *testing.T) { + entity, cleanup := setupTestPatternsEntity(t) + defer cleanup() + + for _, name := range []string{"..", ".foo", "..bar", "~bar", "/abs/path", `\win\path`} { + err := entity.Save(name, []byte("pwned")) + assert.Error(t, err, "expected error for file-path name: %q", name) + if strings.HasPrefix(name, "/") || strings.HasPrefix(name, `\`) { + // If Save accepts an absolute name, it does not write in + // entity.Dir, and the join below points to the incorrect + // location. For these two names, only the error assertion + // gives protection. + continue + } + // For ".." this path is the system.md of the parent directory, + // which Save makes if it accepts a traversal name. + _, statErr := os.Stat(filepath.Join(entity.Dir, name, entity.SystemPatternFile)) + assert.True(t, os.IsNotExist(statErr), "wrote a pattern file for: %q", name) + } +} + +// Rename must reject a file-path-like destination, the same as Save. +// The inherited StorageEntity.Rename accepts such a destination. +func TestPatternsEntity_RenameRejectsFilePathDestination(t *testing.T) { + entity, cleanup := setupTestPatternsEntity(t) + defer cleanup() + + createTestPattern(t, entity, "good-name", "content") + for _, newName := range []string{".foo", "~bar"} { + err := entity.Rename("good-name", newName) + var invalidName *InvalidStorageNameError + require.ErrorAs(t, err, &invalidName, "expected rejection for destination: %q", newName) + _, statErr := os.Stat(filepath.Join(entity.Dir, newName)) + assert.True(t, os.IsNotExist(statErr), "renamed to: %q", newName) + } + + // A valid destination still works. + require.NoError(t, entity.Rename("good-name", "better-name")) + _, err := os.Stat(filepath.Join(entity.Dir, "better-name")) + require.NoError(t, err) +} + +// Save must not write through a symlinked pattern directory or a +// symlinked pattern file that points outside the storage tree. +func TestPatternsEntity_SaveRejectsSymlinkEscape(t *testing.T) { + entity, cleanup := setupTestPatternsEntity(t) + defer cleanup() + + outsideDir := t.TempDir() + mustSymlink(t, outsideDir, filepath.Join(entity.Dir, "linked-dir")) + err := entity.Save("linked-dir", []byte("pwned")) + var invalidName *InvalidStorageNameError + require.ErrorAs(t, err, &invalidName) + _, statErr := os.Stat(filepath.Join(outsideDir, entity.SystemPatternFile)) + assert.True(t, os.IsNotExist(statErr), "wrote through the symlinked pattern dir") + + outsideFile := filepath.Join(outsideDir, "target.md") + require.NoError(t, os.WriteFile(outsideFile, []byte("keep"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(entity.Dir, "real-pattern"), 0o755)) + mustSymlink(t, outsideFile, filepath.Join(entity.Dir, "real-pattern", entity.SystemPatternFile)) + err = entity.Save("real-pattern", []byte("pwned")) + require.ErrorAs(t, err, &invalidName) + got, readErr := os.ReadFile(outsideFile) + require.NoError(t, readErr) + assert.Equal(t, "keep", string(got), "outside pattern file was overwritten") +} + +func TestGetApplyVariables_FromFile(t *testing.T) { + entity, cleanup := setupTestPatternsEntity(t) + defer cleanup() + + path := filepath.Join(t.TempDir(), "fromfile.md") + require.NoError(t, os.WriteFile(path, []byte("Hello {{input}}"), 0o644)) + + result, err := entity.GetApplyVariables(path, nil, "world") + require.NoError(t, err) + assert.Equal(t, "Hello world", result.Pattern) +} + func TestPatternsEntity_CustomPatterns(t *testing.T) { // Create main patterns directory mainDir, err := os.MkdirTemp("", "test-main-patterns-*") @@ -333,20 +417,43 @@ func TestPrintPattern(t *testing.T) { } func TestGetFromDB_PathTraversal(t *testing.T) { + if _, err := i18n.Init("en"); err != nil { + t.Fatalf("i18n.Init() error = %v", err) + } + entity, cleanup := setupTestPatternsEntity(t) defer cleanup() - traversalNames := []string{ - "../etc/passwd", - "../../secret", - "foo/../bar", - "..", - "valid/../../../etc/shadow", + for _, name := range invalidStorageNames { + t.Run(name, func(t *testing.T) { + _, err := entity.GetRaw(name) + require.Error(t, err, "expected error for traversal name: %q", name) + assert.Contains(t, err.Error(), "invalid pattern name", "wrong error for: %q", name) + var invalidName *InvalidStorageNameError + assert.ErrorAs(t, err, &invalidName, "want typed rejection for: %q", name) + }) } - for _, name := range traversalNames { - _, err := entity.GetRaw(name) - assert.Error(t, err, "expected error for traversal name: %q", name) - assert.Contains(t, err.Error(), "invalid pattern name", "wrong error for: %q", name) +} + +// A ".." in a name without separators is one safe path element. +// ValidateStorageName accepts it, and getFromDB also accepts it. +func TestGetFromDB_AllowsDotsWithinName(t *testing.T) { + entity, cleanup := setupTestPatternsEntity(t) + defer cleanup() + + createTestPattern(t, entity, "foo..bar", "dotty {{input}}") + + pattern, err := entity.GetRaw("foo..bar") + require.NoError(t, err) + assert.Equal(t, "dotty {{input}}", pattern.Pattern) +} + +func TestLooksLikePatternFilePath(t *testing.T) { + for _, source := range []string{"/x", `~\x`, `\x`, `.\x`, "~", ".", ".."} { + assert.True(t, LooksLikePatternFilePath(source), "expected file-path detection for: %q", source) + } + for _, source := range []string{"", "pattern", "foo..bar", "a/b", "x~y", "x.y"} { + assert.False(t, LooksLikePatternFilePath(source), "unexpected file-path detection for: %q", source) } } diff --git a/internal/plugins/db/fsdb/sessions.go b/internal/plugins/db/fsdb/sessions.go index f3dd91e6..9a26e1d8 100644 --- a/internal/plugins/db/fsdb/sessions.go +++ b/internal/plugins/db/fsdb/sessions.go @@ -13,6 +13,12 @@ type SessionsEntity struct { } func (o *SessionsEntity) Get(name string) (session *Session, err error) { + // Reject invalid names here. Exists reports false for them, and the + // missing-session branch then answers with a new empty session and + // no error. + if err = ValidateStorageName(name); err != nil { + return nil, err + } session = &Session{Name: name} if o.Exists(name) { diff --git a/internal/plugins/db/fsdb/sessions_test.go b/internal/plugins/db/fsdb/sessions_test.go index 9c2c1d76..c8e7d32d 100644 --- a/internal/plugins/db/fsdb/sessions_test.go +++ b/internal/plugins/db/fsdb/sessions_test.go @@ -21,6 +21,20 @@ func TestSessions_GetOrCreateSession(t *testing.T) { } } +// Get must reject an invalid name and must not answer with a new empty +// session. GET /sessions/ is then a 400, the same as for the +// other entities. +func TestSessions_GetRejectsInvalidNames(t *testing.T) { + sessions := &SessionsEntity{ + StorageEntity: &StorageEntity{Dir: t.TempDir(), FileExtension: ".json"}, + } + for _, name := range invalidStorageNames { + if _, err := sessions.Get(name); err == nil { + t.Errorf("Get(%q) succeeded, want error", name) + } + } +} + func TestSessions_SaveSession(t *testing.T) { dir := t.TempDir() sessions := &SessionsEntity{ diff --git a/internal/plugins/db/fsdb/storage.go b/internal/plugins/db/fsdb/storage.go index 2e9add11..07912052 100644 --- a/internal/plugins/db/fsdb/storage.go +++ b/internal/plugins/db/fsdb/storage.go @@ -11,6 +11,10 @@ import ( "github.com/danielmiessler/fabric/internal/util" ) +// StorageEntity is the filesystem-backed db.Storage implementation. +// Each method that gets a name requires one that obeys +// ValidateStorageName. It rejects other names with +// *InvalidStorageNameError, which HTTP handlers map to 400. type StorageEntity struct { Label string Dir string @@ -68,34 +72,57 @@ func (o *StorageEntity) GetNames() (ret []string, err error) { } func (o *StorageEntity) Delete(name string) (err error) { - if err = os.RemoveAll(o.BuildFilePathByName(name)); err != nil { + var path string + if path, err = o.resolvedPath(name); err != nil { + return + } + if err = os.RemoveAll(path); err != nil { err = fmt.Errorf(i18n.T("storage_error_delete"), name, err) } return } func (o *StorageEntity) Exists(name string) (ret bool) { - _, err := os.Stat(o.BuildFilePathByName(name)) + path, err := o.resolvedPath(name) + if err != nil { + return false + } + _, err = os.Stat(path) ret = !os.IsNotExist(err) return } func (o *StorageEntity) Rename(oldName, newName string) (err error) { - if err = os.Rename(o.BuildFilePathByName(oldName), o.BuildFilePathByName(newName)); err != nil { + var oldPath, newPath string + if oldPath, err = o.resolvedPath(oldName); err != nil { + return + } + if newPath, err = o.resolvedPath(newName); err != nil { + return + } + if err = os.Rename(oldPath, newPath); err != nil { err = fmt.Errorf(i18n.T("storage_error_rename"), oldName, newName, err) } return } func (o *StorageEntity) Save(name string, content []byte) (err error) { - if err = os.WriteFile(o.BuildFilePathByName(name), content, 0644); err != nil { + var path string + if path, err = o.resolvedPath(name); err != nil { + return + } + if err = os.WriteFile(path, content, 0644); err != nil { err = fmt.Errorf(i18n.T("storage_error_save"), name, err) } return } func (o *StorageEntity) Load(name string) (ret []byte, err error) { - if ret, err = os.ReadFile(o.BuildFilePathByName(name)); err != nil { + var path string + if path, err = o.resolvedPath(name); err != nil { + return + } + if ret, err = os.ReadFile(path); err != nil { err = fmt.Errorf(i18n.T("storage_error_load"), name, err) } return @@ -120,11 +147,6 @@ func (o *StorageEntity) ListNames(shellCompleteList bool) (err error) { return } -func (o *StorageEntity) BuildFilePathByName(name string) (ret string) { - ret = o.BuildFilePath(o.buildFileName(name)) - return -} - func (o *StorageEntity) BuildFilePath(fileName string) (ret string) { ret = filepath.Join(o.Dir, fileName) return @@ -134,6 +156,122 @@ func (o *StorageEntity) buildFileName(name string) string { return fmt.Sprintf("%s%v", name, o.FileExtension) } +// InvalidStorageNameError reports a name that storage-name validation +// rejected. HTTP handlers map it to 400 Bad Request. All other storage +// errors stay 500 errors. +type InvalidStorageNameError struct { + Name string + Message string // optional: the default is the storage_invalid_name translation +} + +func (e *InvalidStorageNameError) Error() string { + if e.Message != "" { + return e.Message + } + return fmt.Sprintf(i18n.T("storage_invalid_name"), e.Name) +} + +// windowsReservedNames are DOS device names. On Windows, these names +// identify devices, not files. The match ignores case and all text after +// the first dot, because Windows maps "CON.tar.gz" to the CON device. +var windowsReservedNames = map[string]bool{ + "CON": true, "PRN": true, "AUX": true, "NUL": true, + "COM1": true, "COM2": true, "COM3": true, "COM4": true, "COM5": true, + "COM6": true, "COM7": true, "COM8": true, "COM9": true, + "LPT1": true, "LPT2": true, "LPT3": true, "LPT4": true, "LPT5": true, + "LPT6": true, "LPT7": true, "LPT8": true, "LPT9": true, +} + +// ValidateStorageName rejects an empty name, ".", "..", and each name +// that is not a single path element. It also rejects names that are +// dangerous only on Windows: names with ":" (an NTFS alternate data +// stream suffix), names with a dot or space at the end, and reserved +// DOS device names. Windows removes a dot or space at the end, and the +// shortened name then collides with an existing entry. The policy is +// the same on each platform, and an entry made on one system stays +// valid on the other systems. A Unix +// entry that already has a name against these rules shows in GetNames +// but is not accessible. To repair it, rename its file or directory on +// disk. Call this function before you join a name to a storage +// directory. +func ValidateStorageName(name string) error { + if name == "" || name == "." || name == ".." { + return &InvalidStorageNameError{Name: name} + } + if strings.ContainsAny(name, `/\:`) { + return &InvalidStorageNameError{Name: name} + } + if name != strings.TrimRight(name, ". ") { + return &InvalidStorageNameError{Name: name} + } + base := strings.ToUpper(name) + if i := strings.IndexByte(base, '.'); i >= 0 { + base = base[:i] + } + if windowsReservedNames[base] { + return &InvalidStorageNameError{Name: name} + } + return nil +} + +// symlinkContained rejects an entry at path if the entry resolves, +// through symlinks, to a target outside absDir. A missing entry passes, +// because the lexical check in resolvedPath already keeps the path that +// a write will make in the directory. The two inputs must be absolute +// paths. If they are not, you cannot compare the resolved forms. The +// check does not fully prevent local races. If a hostile local writer +// enters the threat model, move to os.Root. +func symlinkContained(absDir, path, name string) error { + target, err := filepath.EvalSymlinks(path) + if err != nil { + if os.IsNotExist(err) { + if _, lerr := os.Lstat(path); os.IsNotExist(lerr) { + return nil + } + // This is a dangling symlink. A write through it makes the + // outside target. + return &InvalidStorageNameError{Name: name} + } + return err + } + resolvedDir, err := filepath.EvalSymlinks(absDir) + if err != nil { + return err + } + rel, err := filepath.Rel(resolvedDir, target) + if err != nil || !filepath.IsLocal(rel) { + return &InvalidStorageNameError{Name: name} + } + return nil +} + +// resolvedPath keeps name in the entity directory. It validates the +// name, checks containment again after absolute resolution, and +// rejects a symlinked entry that resolves out of the directory. +// Symlinks that stay in the directory are permitted. A storage +// directory that is a symlink is also permitted. +func (o *StorageEntity) resolvedPath(name string) (string, error) { + if err := ValidateStorageName(name); err != nil { + return "", err + } + absDir, err := filepath.Abs(o.Dir) + if err != nil { + return "", err + } + absFull, err := filepath.Abs(filepath.Join(o.Dir, o.buildFileName(name))) + if err != nil { + return "", err + } + rel, err := filepath.Rel(absDir, absFull) + if err != nil || !filepath.IsLocal(rel) { + return "", &InvalidStorageNameError{Name: name} + } + if err := symlinkContained(absDir, absFull, name); err != nil { + return "", err + } + return absFull, nil +} + func (o *StorageEntity) SaveAsJson(name string, item any) (err error) { var jsonString []byte if jsonString, err = json.Marshal(item); err == nil { diff --git a/internal/plugins/db/fsdb/storage_test.go b/internal/plugins/db/fsdb/storage_test.go index 761315e1..e2f20d47 100644 --- a/internal/plugins/db/fsdb/storage_test.go +++ b/internal/plugins/db/fsdb/storage_test.go @@ -1,7 +1,11 @@ package fsdb import ( + "os" + "path/filepath" "testing" + + "github.com/danielmiessler/fabric/internal/i18n" ) func TestStorage_SaveAndLoad(t *testing.T) { @@ -50,3 +54,197 @@ func TestStorage_Delete(t *testing.T) { t.Errorf("expected file to be deleted") } } + +// invalidStorageNames are names that ValidateStorageName must reject on +// each platform. The storage tests and the pattern traversal tests +// share this list, and one new attack name gets a test at each +// location. The backslash cases guard the `\` half of the separator +// check. That half is the Windows-only escape guard that a "simplify +// to filepath.Base" refactor removes without a test failure. The colon, +// reserved-name, and trailing dot and space cases guard the Windows +// protections: NTFS alternate data streams, DOS device names, and name +// suffixes that Windows removes. +var invalidStorageNames = []string{ + "..", "../keep.txt", "/etc/passwd", "foo/../../keep.txt", ".", "", + `foo\bar`, `..\x`, + "foo:bar", "NUL", "con.txt", "CON.tar.gz", "foo.", "foo ", +} + +// newTraversalFixture returns a storage entity in a temporary root and +// a marker file out of the entity directory. It also returns a check +// that fails the test if the marker or the entity directory is gone. +func newTraversalFixture(t *testing.T) (storage *StorageEntity, checkSurvived func()) { + t.Helper() + root := t.TempDir() + dir := filepath.Join(root, "contexts") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + marker := filepath.Join(root, "keep.txt") + if err := os.WriteFile(marker, []byte("keep"), 0o644); err != nil { + t.Fatal(err) + } + storage = &StorageEntity{Dir: dir, Label: "Contexts"} + checkSurvived = func() { + t.Helper() + if _, err := os.Stat(marker); err != nil { + t.Fatalf("parent marker was removed: %v", err) + } + if _, err := os.Stat(dir); err != nil { + t.Fatalf("storage dir was removed: %v", err) + } + } + return +} + +func TestStorage_RejectsPathTraversal(t *testing.T) { + if _, err := i18n.Init("en"); err != nil { + t.Fatalf("i18n.Init() error = %v", err) + } + + storage, checkSurvived := newTraversalFixture(t) + for _, name := range invalidStorageNames { + t.Run(name, func(t *testing.T) { + if err := storage.Delete(name); err == nil { + t.Fatalf("Delete(%q) succeeded, want error", name) + } + if err := storage.Save(name, []byte("pwned")); err == nil { + t.Fatalf("Save(%q) succeeded, want error", name) + } + if _, err := storage.Load(name); err == nil { + t.Fatalf("Load(%q) succeeded, want error", name) + } + if storage.Exists(name) { + t.Fatalf("Exists(%q) is true, want false", name) + } + }) + } + + checkSurvived() +} + +func TestInvalidStorageNameError_DefaultMessage(t *testing.T) { + if _, err := i18n.Init("en"); err != nil { + t.Fatalf("i18n.Init() error = %v", err) + } + err := &InvalidStorageNameError{Name: "bad:name"} + if got, want := err.Error(), `invalid name: "bad:name"`; got != want { + t.Fatalf("Error() = %q, want %q", got, want) + } +} + +func TestStorage_Rename(t *testing.T) { + dir := t.TempDir() + storage := &StorageEntity{Dir: dir, Label: "Contexts"} + if err := storage.Save("old", []byte("content")); err != nil { + t.Fatalf("failed to save content: %v", err) + } + if err := storage.Rename("old", "new"); err != nil { + t.Fatalf("failed to rename: %v", err) + } + if storage.Exists("old") { + t.Errorf("expected old name to be gone") + } + loaded, err := storage.Load("new") + if err != nil { + t.Fatalf("failed to load renamed content: %v", err) + } + if string(loaded) != "content" { + t.Errorf("expected %q, got %q", "content", string(loaded)) + } +} + +func TestStorage_RenameRejectsPathTraversal(t *testing.T) { + if _, err := i18n.Init("en"); err != nil { + t.Fatalf("i18n.Init() error = %v", err) + } + + storage, checkSurvived := newTraversalFixture(t) + if err := storage.Save("ok", []byte("content")); err != nil { + t.Fatalf("failed to save content: %v", err) + } + + for _, name := range invalidStorageNames { + t.Run(name, func(t *testing.T) { + if err := storage.Rename("ok", name); err == nil { + t.Fatalf("Rename(%q, %q) succeeded, want error", "ok", name) + } + if err := storage.Rename(name, "ok"); err == nil { + t.Fatalf("Rename(%q, %q) succeeded, want error", name, "ok") + } + }) + } + + checkSurvived() + if !storage.Exists("ok") { + t.Fatalf("legitimate entry was moved or deleted") + } +} + +// mustSymlink makes a symlink. If symlinks are not available, for +// example on Windows without the privilege, it skips the test. +func mustSymlink(t *testing.T, target, link string) { + t.Helper() + if err := os.Symlink(target, link); err != nil { + t.Skipf("cannot create symlink: %v", err) + } +} + +func TestStorage_RejectsSymlinkEscape(t *testing.T) { + root := t.TempDir() + dir := filepath.Join(root, "store") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + outside := filepath.Join(root, "outside.txt") + if err := os.WriteFile(outside, []byte("keep"), 0o644); err != nil { + t.Fatal(err) + } + mustSymlink(t, outside, filepath.Join(dir, "escape")) + mustSymlink(t, filepath.Join(root, "missing.txt"), filepath.Join(dir, "dangling")) + + storage := &StorageEntity{Dir: dir} + for _, name := range []string{"escape", "dangling"} { + if _, err := storage.Load(name); err == nil { + t.Fatalf("Load(%q) through an outside symlink did not fail", name) + } + if err := storage.Save(name, []byte("pwned")); err == nil { + t.Fatalf("Save(%q) through an outside symlink did not fail", name) + } + } + if got, _ := os.ReadFile(outside); string(got) != "keep" { + t.Fatalf("outside file was overwritten: %q", got) + } + if _, err := os.Stat(filepath.Join(root, "missing.txt")); err == nil { + t.Fatal("dangling symlink target was created") + } +} + +// Load and Save operate through a symlink that stays in the storage +// directory, and through a storage directory that is a symlink. +func TestStorage_AllowsInternalAndDirSymlinks(t *testing.T) { + root := t.TempDir() + realDir := filepath.Join(root, "real") + if err := os.MkdirAll(realDir, 0o755); err != nil { + t.Fatal(err) + } + + storage := &StorageEntity{Dir: realDir} + if err := storage.Save("target", []byte("content")); err != nil { + t.Fatal(err) + } + mustSymlink(t, filepath.Join(realDir, "target"), filepath.Join(realDir, "alias")) + if got, err := storage.Load("alias"); err != nil || string(got) != "content" { + t.Fatalf("Load through an internal symlink: got %q, err %v", got, err) + } + + linkDir := filepath.Join(root, "link") + mustSymlink(t, realDir, linkDir) + linked := &StorageEntity{Dir: linkDir} + if got, err := linked.Load("target"); err != nil || string(got) != "content" { + t.Fatalf("Load via a symlinked storage dir: got %q, err %v", got, err) + } + if err := linked.Save("new", []byte("x")); err != nil { + t.Fatalf("Save via a symlinked storage dir: %v", err) + } +} diff --git a/internal/server/auth.go b/internal/server/auth.go index dfb7d419..bbdc786f 100644 --- a/internal/server/auth.go +++ b/internal/server/auth.go @@ -1,18 +1,47 @@ package restapi import ( + "crypto/sha256" + "crypto/subtle" + "fmt" + "net" "net/http" "strings" + "github.com/danielmiessler/fabric/internal/i18n" "github.com/gin-gonic/gin" ) const APIKeyHeader = "X-API-Key" +// requireAPIKeyForBind rejects a non-loopback bind address that has no +// API key. An empty or unspecified host binds each interface, and that +// counts as non-loopback. +func requireAPIKeyForBind(address, apiKey string) error { + if apiKey != "" { + return nil + } + host := address + if h, _, err := net.SplitHostPort(address); err == nil { + host = h + } + if host == "localhost" { + return nil + } + if ip := net.ParseIP(host); ip != nil && ip.IsLoopback() { + return nil + } + return fmt.Errorf(i18n.T("server_api_key_required"), address) +} + // APIKeyMiddleware validates API key for protected endpoints. // Swagger documentation endpoints (/swagger/*) are exempt from authentication // to allow users to browse and test the API documentation freely. func APIKeyMiddleware(apiKey string) gin.HandlerFunc { + // Compare digests, not the raw values. ConstantTimeCompare returns + // early when the lengths are different, and that shows the length of + // the configured key. + expectedKey := sha256.Sum256([]byte(apiKey)) return func(c *gin.Context) { // Skip authentication for Swagger documentation endpoints // This allows public access to API docs even when authentication is enabled @@ -28,7 +57,8 @@ func APIKeyMiddleware(apiKey string) gin.HandlerFunc { return } - if headerApiKey != apiKey { + headerKey := sha256.Sum256([]byte(headerApiKey)) + if subtle.ConstantTimeCompare(headerKey[:], expectedKey[:]) != 1 { c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "Wrong API Key"}) return } diff --git a/internal/server/chat.go b/internal/server/chat.go index dc3469f7..86cbba5e 100755 --- a/internal/server/chat.go +++ b/internal/server/chat.go @@ -73,11 +73,23 @@ func (h *ChatHandler) HandleChat(c *gin.Context) { if err := c.BindJSON(&request); err != nil { log.Printf("Error binding JSON: %v", err) - c.Writer.Header().Set("Strict-Transport-Security", "max-age=63072000; includeSubDomains") + setHSTS(c) c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf(i18n.T("server_invalid_request_format"), err)}) return } + for _, prompt := range request.Prompts { + if rejectUnsafePatternName(c, prompt.PatternName) { + return + } + if rejectInvalidStorageName(c, prompt.ContextName) { + return + } + if rejectInvalidStorageName(c, prompt.SessionName) { + return + } + } + // Add log to check received language field log.Printf("Received chat request - Language: '%s', Prompts: %d", request.Language, len(request.Prompts)) diff --git a/internal/server/ollama.go b/internal/server/ollama.go index 7b414d10..7d4c062a 100644 --- a/internal/server/ollama.go +++ b/internal/server/ollama.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "log" + "log/slog" "math" "net/http" "net/url" @@ -45,6 +46,7 @@ type APIConvert struct { registry *core.PluginRegistry r *gin.Engine addr *string + apiKey string } type OllamaRequestBody struct { @@ -188,12 +190,49 @@ func parseOllamaNumCtx(options map[string]any) (int, error) { return contextLength, nil } -func ServeOllama(registry *core.PluginRegistry, address string, version string) (err error) { +// fabricChatClient sends the /api/chat self-forward, which can contain +// the configured API key. It does not use a proxy, because the default +// transport obeys HTTP_PROXY and can send the key to the proxy. It does +// not obey redirects, and cannot send the key again to a location that +// the operator did not configure. +var fabricChatClient = newFabricChatClient() + +func newFabricChatClient() *http.Client { + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.Proxy = nil + return &http.Client{ + Transport: transport, + CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }, + } +} + +// ServeOllama operates the Ollama-compatible API server on address. An +// empty apiKey sets authentication to off. This is permitted only for +// loopback binds. +func ServeOllama(registry *core.PluginRegistry, address string, version string, apiKey string) error { + if err := requireAPIKeyForBind(address, apiKey); err != nil { + return err + } + return newOllamaEngine(registry, address, version, apiKey).Run(address) +} + +// newOllamaEngine makes the engine but does not start it, which lets +// tests operate the routes. The address parameter is the /api/chat +// forward target, not the listen address that Run gets. In production +// the two are the same value. +func newOllamaEngine(registry *core.PluginRegistry, address string, version string, apiKey string) *gin.Engine { r := gin.New() // Middleware r.Use(gin.Logger()) r.Use(gin.Recovery()) + if apiKey != "" { + r.Use(APIKeyMiddleware(apiKey)) + } else { + slog.Warn("Starting Ollama-compatible API server without API key authentication. This may pose security risks.") + } // Register routes fabricDb := registry.Db @@ -208,6 +247,7 @@ func ServeOllama(registry *core.PluginRegistry, address string, version string) registry: registry, r: r, addr: &address, + apiKey: apiKey, } // Ollama Endpoints r.GET("/api/tags", typeConversion.ollamaTags) @@ -216,13 +256,7 @@ func ServeOllama(registry *core.PluginRegistry, address string, version string) }) r.POST("/api/chat", typeConversion.ollamaChat) - // Start server - err = r.Run(address) - if err != nil { - return err - } - - return + return r } func (f APIConvert) ollamaTags(c *gin.Context) { @@ -267,7 +301,7 @@ func (f APIConvert) ollamaChat(c *gin.Context) { err = json.Unmarshal(body, &prompt) if err != nil { log.Printf(i18n.T("ollama_error_unmarshalling_body"), err) - c.JSON(http.StatusInternalServerError, gin.H{"error": i18n.T("ollama_error_endpoint")}) + c.JSON(http.StatusBadRequest, gin.H{"error": i18n.T("ollama_invalid_request_body")}) return } @@ -335,14 +369,14 @@ func (f APIConvert) ollamaChat(c *gin.Context) { fabricChatReq, err := json.Marshal(chat) if err != nil { log.Printf(i18n.T("ollama_error_marshalling_body"), err) - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + c.JSON(http.StatusInternalServerError, gin.H{"error": i18n.T("ollama_failed_create_request")}) return } var req *http.Request baseURL, err := buildFabricChatURL(*f.addr) if err != nil { log.Printf(i18n.T("ollama_error_building_chat_url"), err) - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + c.JSON(http.StatusInternalServerError, gin.H{"error": i18n.T("ollama_failed_create_request")}) return } req, err = http.NewRequest("POST", fmt.Sprintf("%s/chat", baseURL), bytes.NewBuffer(fabricChatReq)) @@ -351,13 +385,16 @@ func (f APIConvert) ollamaChat(c *gin.Context) { c.JSON(http.StatusInternalServerError, gin.H{"error": i18n.T("ollama_failed_create_request")}) return } + if f.apiKey != "" { + req.Header.Set(APIKeyHeader, f.apiKey) + } req = req.WithContext(c.Request.Context()) - fabricRes, err := http.DefaultClient.Do(req) + fabricRes, err := fabricChatClient.Do(req) if err != nil { log.Printf(i18n.T("ollama_error_getting_chat_body"), err) - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + c.JSON(http.StatusInternalServerError, gin.H{"error": i18n.T("ollama_upstream_request_failed")}) return } defer fabricRes.Body.Close() diff --git a/internal/server/ollama_test.go b/internal/server/ollama_test.go index 0af11f9e..c47a6ca7 100644 --- a/internal/server/ollama_test.go +++ b/internal/server/ollama_test.go @@ -2,9 +2,16 @@ package restapi import ( "encoding/json" + "fmt" "math" + "net/http" + "net/http/httptest" "strings" "testing" + + "github.com/danielmiessler/fabric/internal/core" + "github.com/danielmiessler/fabric/internal/plugins/db/fsdb" + "github.com/gin-gonic/gin" ) func TestBuildFabricChatURL(t *testing.T) { @@ -361,3 +368,193 @@ func TestParseOllamaNumCtx(t *testing.T) { }) } } + +func TestNewOllamaEngine_APIKeyWiring(t *testing.T) { + gin.SetMode(gin.TestMode) + registry := &core.PluginRegistry{Db: fsdb.NewDb(t.TempDir())} + + getVersion := func(r *gin.Engine, key string) int { + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/api/version", nil) + if key != "" { + req.Header.Set(APIKeyHeader, key) + } + r.ServeHTTP(w, req) + return w.Code + } + + withKey := newOllamaEngine(registry, ":0", "test-version", "secret") + if code := getVersion(withKey, ""); code != http.StatusUnauthorized { + t.Fatalf("no key presented: got %d, want 401", code) + } + if code := getVersion(withKey, "secret"); code != http.StatusOK { + t.Fatalf("valid key presented: got %d, want 200", code) + } + + withoutKey := newOllamaEngine(registry, ":0", "test-version", "") + if code := getVersion(withoutKey, ""); code != http.StatusOK { + t.Fatalf("no key configured: got %d, want 200", code) + } +} + +func TestOllamaChat_ForwardsAPIKeyToChat(t *testing.T) { + gin.SetMode(gin.TestMode) + + // Make the loopback /chat route with the middleware installed, the + // same as newOllamaEngine makes it when --api-key is set. + upstream := gin.New() + upstream.Use(APIKeyMiddleware("secret")) + upstream.POST("/chat", func(c *gin.Context) { + c.Writer.Header().Set("Content-Type", "text/event-stream") + fmt.Fprint(c.Writer, "data: {\"type\":\"content\",\"format\":\"markdown\",\"content\":\"hi\"}\n\n") + }) + server := httptest.NewServer(upstream) + defer server.Close() + + chatRequest := func(key string) int { + r := gin.New() + conv := APIConvert{addr: &server.URL, apiKey: key} + r.POST("/api/chat", conv.ollamaChat) + + w := httptest.NewRecorder() + body := `{"model":"test:latest","messages":[{"role":"user","content":"hi"}]}` + req := httptest.NewRequest(http.MethodPost, "/api/chat", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + return w.Code + } + + if code := chatRequest("secret"); code != http.StatusOK { + t.Fatalf("matching key: got %d, want 200", code) + } + if code := chatRequest("wrong"); code != http.StatusUnauthorized { + t.Fatalf("wrong key: got %d, want 401", code) + } +} + +// The self-forward client must not use a proxy. A proxy gets the +// configured API key, and the operator did not configure that host. +// The redirect test below covers the no-redirect property. +func TestFabricChatClient_NoProxy(t *testing.T) { + transport, ok := fabricChatClient.Transport.(*http.Transport) + if !ok { + t.Fatalf("transport is %T, want *http.Transport", fabricChatClient.Transport) + } + if transport.Proxy != nil { + t.Fatal("self-forward transport has a proxy configured") + } +} + +// An upstream redirect must show as an upstream error. The client must +// not go to the redirect target with the API key. +func TestOllamaChat_DoesNotFollowUpstreamRedirect(t *testing.T) { + gin.SetMode(gin.TestMode) + + redirectTargetHit := false + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + redirectTargetHit = true + })) + defer target.Close() + + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, target.URL+"/chat", http.StatusFound) + })) + defer upstream.Close() + + r := gin.New() + conv := APIConvert{addr: &upstream.URL, apiKey: "secret"} + r.POST("/api/chat", conv.ollamaChat) + + w := httptest.NewRecorder() + body := `{"model":"test:latest","messages":[{"role":"user","content":"hi"}]}` + req := httptest.NewRequest(http.MethodPost, "/api/chat", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + + if redirectTargetHit { + t.Fatal("the redirect target was contacted") + } + if w.Code != http.StatusFound { + t.Fatalf("got %d, want the upstream 302 surfaced as an error", w.Code) + } +} + +// Malformed client JSON is a client error. The answer is a 400 with a +// stable generic message, not a 500. +func TestOllamaChat_MalformedJSONIs400(t *testing.T) { + gin.SetMode(gin.TestMode) + + addr := ":0" + r := gin.New() + conv := APIConvert{addr: &addr} + r.POST("/api/chat", conv.ollamaChat) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/api/chat", strings.NewReader("{not json")) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Fatalf("got %d, want 400", w.Code) + } +} + +// An upstream that is not available is a 500. The body must not contain +// the raw transport error, because that error shows the internal +// upstream URL. +func TestOllamaChat_UpstreamFailureHidesDetails(t *testing.T) { + gin.SetMode(gin.TestMode) + + server := httptest.NewServer(http.NotFoundHandler()) + url := server.URL + server.Close() // nothing listens on url anymore + + r := gin.New() + conv := APIConvert{addr: &url} + r.POST("/api/chat", conv.ollamaChat) + + w := httptest.NewRecorder() + body := `{"model":"test:latest","messages":[{"role":"user","content":"hi"}]}` + req := httptest.NewRequest(http.MethodPost, "/api/chat", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + + if w.Code != http.StatusInternalServerError { + t.Fatalf("got %d, want 500", w.Code) + } + got := w.Body.String() + if strings.Contains(got, "dial tcp") || strings.Contains(got, strings.TrimPrefix(url, "http://")) { + t.Fatalf("500 body leaks transport details: %s", got) + } +} + +// With no configured key, the forwarded request must not contain the header. +func TestOllamaChat_NoKeyOmitsHeader(t *testing.T) { + gin.SetMode(gin.TestMode) + + var gotHeader string + upstream := gin.New() + upstream.POST("/chat", func(c *gin.Context) { + gotHeader = c.GetHeader(APIKeyHeader) + c.Writer.Header().Set("Content-Type", "text/event-stream") + fmt.Fprint(c.Writer, "data: {\"type\":\"content\",\"format\":\"markdown\",\"content\":\"hi\"}\n\n") + }) + server := httptest.NewServer(upstream) + defer server.Close() + + r := gin.New() + conv := APIConvert{addr: &server.URL, apiKey: ""} + r.POST("/api/chat", conv.ollamaChat) + + w := httptest.NewRecorder() + body := `{"model":"test:latest","messages":[{"role":"user","content":"hi"}]}` + req := httptest.NewRequest(http.MethodPost, "/api/chat", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("got %d, want 200", w.Code) + } + if gotHeader != "" { + t.Fatalf("X-API-Key was forwarded with no key configured: %q", gotHeader) + } +} diff --git a/internal/server/path_traversal_test.go b/internal/server/path_traversal_test.go new file mode 100644 index 00000000..390f011f --- /dev/null +++ b/internal/server/path_traversal_test.go @@ -0,0 +1,330 @@ +package restapi + +import ( + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/danielmiessler/fabric/internal/i18n" + "github.com/danielmiessler/fabric/internal/plugins/db/fsdb" + "github.com/gin-gonic/gin" +) + +// Each storage route that gets a name must reject a traversal name, not +// only DELETE. Most routes share storageError through the fsdb layer. +// The exists route validates in the handler, because its storage +// contract returns only a bool. +func TestStorageHandler_RejectsTraversalOnAllRoutes(t *testing.T) { + if _, err := i18n.Init("en"); err != nil { + t.Fatalf("i18n.Init() error = %v", err) + } + gin.SetMode(gin.TestMode) + + root := t.TempDir() + contextsDir := filepath.Join(root, "contexts") + if err := os.MkdirAll(contextsDir, 0o755); err != nil { + t.Fatal(err) + } + marker := filepath.Join(root, "keep.txt") + if err := os.WriteFile(marker, []byte("keep"), 0o644); err != nil { + t.Fatal(err) + } + + r := gin.New() + NewContextsHandler(r, &fsdb.ContextsEntity{ + StorageEntity: &fsdb.StorageEntity{Label: "Contexts", Dir: contextsDir}, + }) + + for _, tc := range []struct{ method, path string }{ + {http.MethodGet, "/contexts/%2e%2e"}, + {http.MethodDelete, "/contexts/%2e%2e"}, + {http.MethodDelete, "/contexts/.."}, + {http.MethodPost, "/contexts/%2e%2e"}, + {http.MethodPut, "/contexts/rename/%2e%2e/ok"}, + {http.MethodPut, "/contexts/rename/ok/%2e%2e"}, + {http.MethodGet, "/contexts/exists/%2e%2e"}, + } { + var body io.Reader + if tc.method == http.MethodPost { + body = strings.NewReader("x") + } + w := httptest.NewRecorder() + req := httptest.NewRequest(tc.method, tc.path, body) + r.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Fatalf("%s %s: got %d, want 400", tc.method, tc.path, w.Code) + } + if w.Header().Get("Strict-Transport-Security") == "" { + t.Fatalf("%s %s: validation 400 lacks the HSTS header", tc.method, tc.path) + } + } + + if _, err := os.Stat(marker); err != nil { + t.Fatalf("parent marker was deleted: %v", err) + } + if _, err := os.Stat(contextsDir); err != nil { + t.Fatalf("contexts dir was deleted: %v", err) + } +} + +// A non-validation failure stays a 500. Its body must not show the +// filesystem path that is in the wrapped *os.PathError. +func TestStorageHandler_GenericErrorHidesPaths(t *testing.T) { + if _, err := i18n.Init("en"); err != nil { + t.Fatalf("i18n.Init() error = %v", err) + } + gin.SetMode(gin.TestMode) + + dir := t.TempDir() + r := gin.New() + NewContextsHandler(r, &fsdb.ContextsEntity{ + StorageEntity: &fsdb.StorageEntity{Label: "Contexts", Dir: dir}, + }) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/contexts/missing", nil) + r.ServeHTTP(w, req) + if w.Code != http.StatusInternalServerError { + t.Fatalf("got %d, want 500", w.Code) + } + if strings.Contains(w.Body.String(), dir) { + t.Fatalf("500 body leaks the storage path: %s", w.Body.String()) + } + if !strings.Contains(w.Body.String(), "internal error") { + t.Fatalf("500 body is not the generic message: %s", w.Body.String()) + } +} + +// A pattern backend failure must use the shared storageError mapping. +// That is a JSON envelope with the generic message, never the wrapped +// os.PathError. +func TestPatternsHandler_BackendErrorHidesPaths(t *testing.T) { + if _, err := i18n.Init("en"); err != nil { + t.Fatalf("i18n.Init() error = %v", err) + } + gin.SetMode(gin.TestMode) + + dir := t.TempDir() + r := gin.New() + NewPatternsHandler(r, &fsdb.PatternsEntity{ + StorageEntity: &fsdb.StorageEntity{Label: "Patterns", Dir: dir, ItemIsDir: true}, + SystemPatternFile: "system.md", + }) + + for _, req := range []*http.Request{ + httptest.NewRequest(http.MethodGet, "/patterns/missing", nil), + httptest.NewRequest(http.MethodPost, "/patterns/missing/apply", strings.NewReader(`{"input":"x"}`)), + } { + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusInternalServerError { + t.Fatalf("%s %s: got %d, want 500", req.Method, req.URL.Path, w.Code) + } + if strings.Contains(w.Body.String(), dir) { + t.Fatalf("%s %s: 500 body leaks the storage path: %s", req.Method, req.URL.Path, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "internal error") { + t.Fatalf("%s %s: 500 body is not the generic envelope: %s", req.Method, req.URL.Path, w.Body.String()) + } + } +} + +func TestPatternsHandler_RejectsPathTraversalSave(t *testing.T) { + if _, err := i18n.Init("en"); err != nil { + t.Fatalf("i18n.Init() error = %v", err) + } + gin.SetMode(gin.TestMode) + + root := t.TempDir() + patternsDir := filepath.Join(root, "patterns") + if err := os.MkdirAll(patternsDir, 0o755); err != nil { + t.Fatal(err) + } + + r := gin.New() + NewPatternsHandler(r, &fsdb.PatternsEntity{ + StorageEntity: &fsdb.StorageEntity{Label: "Patterns", Dir: patternsDir, ItemIsDir: true}, + SystemPatternFile: "system.md", + }) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/patterns/%2e%2e", strings.NewReader("pwned")) + r.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Fatalf("POST /patterns/%%2e%%2e: got %d, want 400", w.Code) + } + if _, err := os.Stat(filepath.Join(root, "system.md")); err == nil { + t.Fatal("wrote system.md in the parent directory") + } +} + +func TestChatHandler_RejectsUnsafeNames(t *testing.T) { + if _, err := i18n.Init("en"); err != nil { + t.Fatalf("i18n.Init() error = %v", err) + } + gin.SetMode(gin.TestMode) + + // The zero-value handler is an intentional seam. A request that goes + // through pre-validation causes a nil panic in HandleChat. These + // tests cannot pass on a request that got no validation. + r := gin.New() + r.POST("/chat", (&ChatHandler{}).HandleChat) + + cases := map[string]string{ + // Every prefix class loadPattern treats as a filesystem path + "absolute pattern name": `{"prompts":[{"patternName":"/etc/hosts","userInput":"x"}]}`, + "home pattern name": `{"prompts":[{"patternName":"~/secret.md","userInput":"x"}]}`, + "relative pattern name": `{"prompts":[{"patternName":"./secret.md","userInput":"x"}]}`, + "backslash pattern name": `{"prompts":[{"patternName":"\\secret.md","userInput":"x"}]}`, + // Context and session names get the same pre-validation + "traversal context name": `{"prompts":[{"userInput":"x","contextName":"../keep.txt"}]}`, + "traversal session name": `{"prompts":[{"userInput":"x","sessionName":".."}]}`, + // The rejection loop stops at the first bad name, at each depth + "second prompt path-like": `{"prompts":[{"userInput":"x"},{"patternName":"/etc/hosts","userInput":"y"}]}`, + } + for name, body := range cases { + t.Run(name, func(t *testing.T) { + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/chat", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Fatalf("got status %d, want 400", w.Code) + } + }) + } +} + +func TestPatternsHandler_RejectsUnsafeNamesOnReadRoutes(t *testing.T) { + if _, err := i18n.Init("en"); err != nil { + t.Fatalf("i18n.Init() error = %v", err) + } + gin.SetMode(gin.TestMode) + + r := gin.New() + NewPatternsHandler(r, &fsdb.PatternsEntity{ + StorageEntity: &fsdb.StorageEntity{Label: "Patterns", Dir: t.TempDir(), ItemIsDir: true}, + SystemPatternFile: "system.md", + }) + + for _, req := range []*http.Request{ + httptest.NewRequest(http.MethodGet, "/patterns/%2e%2e", nil), + httptest.NewRequest(http.MethodPost, "/patterns/%2e%2e/apply", strings.NewReader(`{"input":"x"}`)), + // Names that fail only ValidateStorageName, not the file-path check + httptest.NewRequest(http.MethodGet, "/patterns/foo:bar", nil), + httptest.NewRequest(http.MethodGet, "/patterns/NUL", nil), + httptest.NewRequest(http.MethodPost, "/patterns/foo:bar/apply", strings.NewReader(`{"input":"x"}`)), + } { + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Fatalf("%s %s: got %d, want 400", req.Method, req.URL.Path, w.Code) + } + } +} + +// A safe request must go through pre-validation. With the zero-value +// handler seam, a request that goes through causes a nil panic, and +// Recovery changes that into a 500. A 400 shows that validation +// rejected valid names. +func TestChatHandler_AcceptsBenignNames(t *testing.T) { + if _, err := i18n.Init("en"); err != nil { + t.Fatalf("i18n.Init() error = %v", err) + } + gin.SetMode(gin.TestMode) + + r := gin.New() + r.Use(gin.Recovery()) + r.POST("/chat", (&ChatHandler{}).HandleChat) + + w := httptest.NewRecorder() + body := `{"prompts":[{"userInput":"x","patternName":"summarize","contextName":"myctx","sessionName":"mysession"}]}` + req := httptest.NewRequest(http.MethodPost, "/chat", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + if w.Code == http.StatusBadRequest { + t.Fatalf("benign request was rejected with 400: %s", w.Body.String()) + } +} + +// A non-loopback bind without an API key must fail closed before the +// server starts. A loopback bind operates without a key. +func TestRequireAPIKeyForBind(t *testing.T) { + tests := []struct { + address string + apiKey string + wantErr bool + }{ + {"127.0.0.1:8080", "", false}, + {"localhost:8080", "", false}, + {"localhost", "", false}, + {"[::1]:8080", "", false}, + {":8080", "", true}, // wildcard bind exposes every interface + {"0.0.0.0:8080", "", true}, + {"[::]:8080", "", true}, + {"192.168.1.50:8080", "", true}, + {"example.com:8080", "", true}, + {":8080", "secret", false}, + {"0.0.0.0:8080", "secret", false}, + } + for _, tt := range tests { + t.Run(tt.address, func(t *testing.T) { + err := requireAPIKeyForBind(tt.address, tt.apiKey) + if (err != nil) != tt.wantErr { + t.Fatalf("requireAPIKeyForBind(%q, %q) error = %v, wantErr %v", tt.address, tt.apiKey, err, tt.wantErr) + } + }) + } +} + +// Serve and ServeOllama must return the fail-closed error and must not +// start an unauthenticated server on a non-loopback bind. The registry +// is nil, and a check that does not occur first causes a panic. +func TestServeFailsClosedOnNonLoopbackBind(t *testing.T) { + if err := Serve(nil, ":0", ""); err == nil { + t.Fatal("Serve on a wildcard bind without a key did not fail") + } + if err := ServeOllama(nil, ":0", "v", ""); err == nil { + t.Fatal("ServeOllama on a wildcard bind without a key did not fail") + } +} + +func TestAPIKeyMiddleware(t *testing.T) { + gin.SetMode(gin.TestMode) + r := gin.New() + r.Use(APIKeyMiddleware("secret")) + r.GET("/ping", func(c *gin.Context) { c.Status(http.StatusOK) }) + + t.Run("missing key", func(t *testing.T) { + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/ping", nil) + r.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + t.Fatalf("got %d, want 401", w.Code) + } + }) + + t.Run("wrong key", func(t *testing.T) { + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/ping", nil) + req.Header.Set(APIKeyHeader, "wrong") + r.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + t.Fatalf("got %d, want 401", w.Code) + } + }) + + t.Run("valid key", func(t *testing.T) { + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/ping", nil) + req.Header.Set(APIKeyHeader, "secret") + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("got %d, want 200", w.Code) + } + }) +} diff --git a/internal/server/patterns.go b/internal/server/patterns.go index 5b75b67e..ad9c9d53 100644 --- a/internal/server/patterns.go +++ b/internal/server/patterns.go @@ -1,9 +1,11 @@ package restapi import ( + "fmt" "maps" "net/http" + "github.com/danielmiessler/fabric/internal/i18n" "github.com/danielmiessler/fabric/internal/plugins/db/fsdb" "github.com/gin-gonic/gin" ) @@ -14,6 +16,22 @@ type PatternsHandler struct { patterns *fsdb.PatternsEntity } +// rejectUnsafePatternName answers a 400 when name is a file-path-like +// pattern name or does not obey storage-name validation. An empty name +// passes, because the chat handler guards prompt.PatternName, which is +// optional. +func rejectUnsafePatternName(c *gin.Context, name string) bool { + if name == "" { + return false + } + if fsdb.LooksLikePatternFilePath(name) || fsdb.ValidateStorageName(name) != nil { + setHSTS(c) + c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf(i18n.T("pattern_invalid_name"), name)}) + return true + } + return false +} + // NewPatternsHandler creates a new PatternsHandler func NewPatternsHandler(r *gin.Engine, patterns *fsdb.PatternsEntity) (ret *PatternsHandler) { // Create a storage handler but don't register any routes yet @@ -40,15 +58,19 @@ func NewPatternsHandler(r *gin.Engine, patterns *fsdb.PatternsEntity) (ret *Patt // @Produce json // @Param name path string true "Pattern name" // @Success 200 {object} fsdb.Pattern +// @Failure 400 {object} map[string]string // @Failure 500 {object} map[string]string // @Security ApiKeyAuth // @Router /patterns/{name} [get] func (h *PatternsHandler) Get(c *gin.Context) { name := c.Param("name") + if rejectUnsafePatternName(c, name) { + return + } pattern, err := h.patterns.GetRaw(name) if err != nil { - c.JSON(http.StatusInternalServerError, err.Error()) + storageError(c, err) return } c.JSON(http.StatusOK, pattern) @@ -75,6 +97,9 @@ type PatternApplyRequest struct { // @Router /patterns/{name}/apply [post] func (h *PatternsHandler) ApplyPattern(c *gin.Context) { name := c.Param("name") + if rejectUnsafePatternName(c, name) { + return + } var request PatternApplyRequest if err := c.ShouldBindJSON(&request); err != nil { @@ -93,7 +118,7 @@ func (h *PatternsHandler) ApplyPattern(c *gin.Context) { pattern, err := h.patterns.GetApplyVariables(name, variables, request.Input) if err != nil { - c.JSON(http.StatusInternalServerError, err.Error()) + storageError(c, err) return } c.JSON(http.StatusOK, pattern) diff --git a/internal/server/serve.go b/internal/server/serve.go index 15eea494..35c55f94 100644 --- a/internal/server/serve.go +++ b/internal/server/serve.go @@ -7,6 +7,7 @@ import ( "path/filepath" "github.com/danielmiessler/fabric/internal/core" + "github.com/danielmiessler/fabric/internal/i18n" "github.com/gin-gonic/gin" swaggerFiles "github.com/swaggo/files" ginSwagger "github.com/swaggo/gin-swagger" @@ -27,6 +28,10 @@ import ( // @in header // @name X-API-Key func Serve(registry *core.PluginRegistry, address string, apiKey string) (err error) { + if err = requireAPIKeyForBind(address, apiKey); err != nil { + return err + } + r := gin.New() // Middleware @@ -36,7 +41,7 @@ func Serve(registry *core.PluginRegistry, address string, apiKey string) (err er if apiKey != "" { r.Use(APIKeyMiddleware(apiKey)) } else { - slog.Warn("Starting REST API server without API key authentication. This may pose security risks.") + slog.Warn(i18n.T("server_no_api_key_warning")) } // Swagger UI and documentation endpoint with custom YAML handler diff --git a/internal/server/storage.go b/internal/server/storage.go index 40e402d6..eda8fb77 100644 --- a/internal/server/storage.go +++ b/internal/server/storage.go @@ -1,11 +1,14 @@ package restapi import ( + "errors" "fmt" "io" + "log/slog" "net/http" "github.com/danielmiessler/fabric/internal/plugins/db" + "github.com/danielmiessler/fabric/internal/plugins/db/fsdb" "github.com/gin-gonic/gin" ) @@ -14,6 +17,42 @@ type StorageHandler[T any] struct { storage db.Storage[T] } +// setHSTS sets the Strict-Transport-Security header. Each validation +// 400 sends it, the same as the chat BindJSON 400 path. +func setHSTS(c *gin.Context) { + c.Writer.Header().Set("Strict-Transport-Security", "max-age=63072000; includeSubDomains") +} + +// storageError answers err. A name-validation rejection becomes a 400, +// and its body contains only the rejected name. All other errors stay +// 500 errors with a generic body, because fsdb wraps *os.PathError +// values and err.Error() then sends absolute filesystem paths to the +// client. The full error goes to the log. +func storageError(c *gin.Context, err error) { + if _, ok := errors.AsType[*fsdb.InvalidStorageNameError](err); ok { + setHSTS(c) + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + slog.Error("storage operation failed", "error", err) + c.JSON(http.StatusInternalServerError, gin.H{"error": "internal error"}) +} + +// rejectInvalidStorageName answers a 400 when name does not obey +// storage-name validation. An empty name passes, because the fields +// that this guards are optional. +func rejectInvalidStorageName(c *gin.Context, name string) bool { + if name == "" { + return false + } + if err := fsdb.ValidateStorageName(name); err != nil { + setHSTS(c) + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return true + } + return false +} + // NewStorageHandler creates a new StorageHandler func NewStorageHandler[T any](r *gin.Engine, entityType string, storage db.Storage[T]) (ret *StorageHandler[T]) { ret = &StorageHandler[T]{storage: storage} @@ -31,7 +70,7 @@ func (h *StorageHandler[T]) Get(c *gin.Context) { name := c.Param("name") item, err := h.storage.Get(name) if err != nil { - c.JSON(http.StatusInternalServerError, err.Error()) + storageError(c, err) return } c.JSON(http.StatusOK, item) @@ -41,7 +80,7 @@ func (h *StorageHandler[T]) Get(c *gin.Context) { func (h *StorageHandler[T]) GetNames(c *gin.Context) { names, err := h.storage.GetNames() if err != nil { - c.JSON(http.StatusInternalServerError, err.Error()) + storageError(c, err) return } c.JSON(http.StatusOK, names) @@ -52,15 +91,20 @@ func (h *StorageHandler[T]) Delete(c *gin.Context) { name := c.Param("name") err := h.storage.Delete(name) if err != nil { - c.JSON(http.StatusInternalServerError, err.Error()) + storageError(c, err) return } c.Status(http.StatusOK) } -// Exists handles the GET /storage/exists/:name route +// Exists handles the GET /storage/exists/:name route. The storage +// Exists contract cannot report an invalid name, and the handler must +// validate the name itself. An invalid name is a 400, not a "false". func (h *StorageHandler[T]) Exists(c *gin.Context) { name := c.Param("name") + if rejectInvalidStorageName(c, name) { + return + } exists := h.storage.Exists(name) c.JSON(http.StatusOK, exists) } @@ -71,7 +115,7 @@ func (h *StorageHandler[T]) Rename(c *gin.Context) { newName := c.Param("newName") err := h.storage.Rename(oldName, newName) if err != nil { - c.JSON(http.StatusInternalServerError, err.Error()) + storageError(c, err) return } c.Status(http.StatusOK) @@ -87,14 +131,14 @@ func (h *StorageHandler[T]) Save(c *gin.Context) { content, err := io.ReadAll(body) if err != nil { - c.JSON(http.StatusInternalServerError, err.Error()) + storageError(c, err) return } // Save the content to storage err = h.storage.Save(name, content) if err != nil { - c.JSON(http.StatusInternalServerError, err.Error()) + storageError(c, err) return } c.Status(http.StatusOK) diff --git a/scripts/docker/README.md b/scripts/docker/README.md index a21f1f4b..d8c5ea4b 100644 --- a/scripts/docker/README.md +++ b/scripts/docker/README.md @@ -39,13 +39,17 @@ docker run --rm -it -v $PWD/.env:/root/.config/fabric/.env fabric -p your-patter ## Running the server -Expose port 8080 to use Fabric's REST API: +Expose port 8080 to use Fabric's REST API. In a container, bind all +interfaces with `--address :8080` so the mapped port can reach the +server, and set an API key, which is mandatory for non-loopback binds: ```bash -docker run --rm -it -p 8080:8080 -v $HOME/.fabric-config:/root/.config/fabric fabric --serve +docker run --rm -it -p 8080:8080 -v $HOME/.fabric-config:/root/.config/fabric \ + -e FABRIC_API_KEY=your-secret-key fabric --serve --address :8080 ``` -The API will be available at `http://localhost:8080`. +The API will be available at `http://localhost:8080`. Requests must send +the key in the `X-API-Key` header. ## Multi-arch builds and GHCR packages