diff --git a/adminapi/auth/auth_handler.go b/adminapi/auth/auth_handler.go index 802995d..449cb9f 100644 --- a/adminapi/auth/auth_handler.go +++ b/adminapi/auth/auth_handler.go @@ -119,6 +119,7 @@ func BasicAuthHandler(w http.ResponseWriter, r *http.Request) { if err != nil { log.Error("Authentication Error : ", err) http.Error(w, "Authentication Error", http.StatusUnauthorized) + return } // Add the cookie to the response w.Header()[xhttp.AUTH_TOKEN] = []string{token} @@ -129,6 +130,7 @@ func BasicAuthHandler(w http.ResponseWriter, r *http.Request) { xwhttp.WriteXconfResponseWithHeaders(w, headers, http.StatusFound, []byte("")) } else { http.Error(w, "Unauthorized", http.StatusUnauthorized) + return } } diff --git a/adminapi/canary/canary_settings_handler.go b/adminapi/canary/canary_settings_handler.go index 0081e3e..1ff0261 100644 --- a/adminapi/canary/canary_settings_handler.go +++ b/adminapi/canary/canary_settings_handler.go @@ -32,6 +32,7 @@ import ( func PutCanarySettingsHandler(w http.ResponseWriter, r *http.Request) { if !auth.HasWritePermissionForTool(r) { xhttp.WriteAdminErrorResponse(w, http.StatusForbidden, "No write permission: tools") + return } xw, ok := w.(*xwhttp.XResponseWriter) diff --git a/adminapi/canary/canary_settings_handler_test.go b/adminapi/canary/canary_settings_handler_test.go index fbe4a62..c799675 100644 --- a/adminapi/canary/canary_settings_handler_test.go +++ b/adminapi/canary/canary_settings_handler_test.go @@ -74,3 +74,36 @@ func TestGetCanarySettingsHandler(t *testing.T) { GetCanarySettingsHandler(w, req) assert.Equal(t, http.StatusUnauthorized, w.Status()) } + +// TestAuthFailureTerminatesExecution verifies the fail-fast termination guarantee: +// When an auth/authz error response is written, the handler MUST return immediately +// with no subsequent logic execution (Fail-Fast Termination per auth-contract.md). +func TestAuthFailureTerminatesExecution(t *testing.T) { + originalSatOn := common.SatOn + defer func() { common.SatOn = originalSatOn }() + + // Enable SAT to trigger permission check + common.SatOn = true + + // Create a request without authentication credentials (will fail permission check) + req := httptest.NewRequest(http.MethodPut, testURL, nil) + + // Provide invalid JSON that would normally cause http.StatusBadRequest + // if the handler continued executing past the auth failure. + // However, with fail-fast termination, the handler should return + // http.StatusForbidden from the permission check and never reach JSON parsing. + recorder := httptest.NewRecorder() + w := xwhttp.NewXResponseWriter(recorder) + w.SetBody(`{"invalid": json}`) // Malformed JSON + + // Call the handler + PutCanarySettingsHandler(w, req) + + // CRITICAL ASSERTION: Verify fail-fast termination + // Status should be http.StatusForbidden (from auth failure), + // NOT http.StatusBadRequest (which would occur if JSON parsing ran). + assert.Equal(t, http.StatusForbidden, w.Status(), + "Auth failure must return Forbidden and terminate immediately. "+ + "If JSON parsing error (BadRequest) occurs instead, "+ + "handler continued executing after auth failure, violating fail-fast guarantee.") +} diff --git a/adminapi/lockdown/lockdown_settings_handler.go b/adminapi/lockdown/lockdown_settings_handler.go index dd1141d..6151ce6 100644 --- a/adminapi/lockdown/lockdown_settings_handler.go +++ b/adminapi/lockdown/lockdown_settings_handler.go @@ -30,6 +30,7 @@ import ( func PutLockdownSettingsHandler(w http.ResponseWriter, r *http.Request) { if !auth.HasWritePermissionForTool(r) { xhttp.WriteAdminErrorResponse(w, http.StatusForbidden, "No write permission: tools") + return } xw, ok := w.(*xhttp.XResponseWriter) diff --git a/adminapi/queries/common.go b/adminapi/queries/common.go index e111ff7..1b7e72e 100644 --- a/adminapi/queries/common.go +++ b/adminapi/queries/common.go @@ -394,6 +394,7 @@ func UpdateAppSettings(w http.ResponseWriter, r *http.Request) { // For updating app settings, tools permission is required if !auth.HasWritePermissionForTool(r) { xhttp.WriteAdminErrorResponse(w, http.StatusUnauthorized, "") + return } // r.Body is already drained in the middleware diff --git a/openspec/changes/enforce-auth-failure-termination/.openspec.yaml b/openspec/changes/enforce-auth-failure-termination/.openspec.yaml new file mode 100644 index 0000000..3e307b4 --- /dev/null +++ b/openspec/changes/enforce-auth-failure-termination/.openspec.yaml @@ -0,0 +1,6 @@ +schema: spec-driven +created: 2026-04-23 +status: accepted +accepted: 2026-04-27 +applies_to: + - openspec/specs/auth/auth-contract.md \ No newline at end of file diff --git a/openspec/changes/enforce-auth-failure-termination/proposal.md b/openspec/changes/enforce-auth-failure-termination/proposal.md new file mode 100644 index 0000000..aeb18fc --- /dev/null +++ b/openspec/changes/enforce-auth-failure-termination/proposal.md @@ -0,0 +1,35 @@ +Status: Accepted +Applied to: openspec/specs/auth/auth-contract.md + +## Why + +Authentication and authorization failures currently rely on handler authors to stop execution after an authentication or authorization failure is produced, which is easy to miss and can allow unintended logic to continue. We need an explicit contract that fail-fast termination is mandatory whenever an auth/authz failure is produced. + +## What Changes + +- Clarify the auth contract to require that request processing terminates immediately after an authentication or authorization failure is produced. +- Define this behavior as a normative guarantee so downstream handlers and middleware can rely on no post-failure side effects. +- Align xconfadmin auth behavior documentation with this fail-fast requirement so future implementations and refactors preserve the same safety property. + +## Non‑Goals + +- This change does not alter authentication mechanisms, token types, + permission models, or authorization semantics. +- This change does not introduce new authentication or authorization + capabilities. +- This change does not modify request/response formats or API contracts. + +## Capabilities + +### New Capabilities +- None. + +### Modified Capabilities +- `auth`: Add explicit fail-fast requirements that handler execution MUST NOT continue after authentication or authorization failures are identified. + +## Impact + +- Affected specs: `openspec/specs/auth/auth-contract.md`. +- Affected implementation areas (future apply phase): auth middleware and handlers that write 401/403/related auth error responses. +- API behavior impact: no new endpoints; clarifies control-flow guarantees for existing auth/authz failure paths. +- System impact: improves safety by preventing accidental post-failure processing and side effects. \ No newline at end of file diff --git a/openspec/config.yaml b/openspec/config.yaml new file mode 100644 index 0000000..392946c --- /dev/null +++ b/openspec/config.yaml @@ -0,0 +1,20 @@ +schema: spec-driven + +# Project context (optional) +# This is shown to AI when creating artifacts. +# Add your tech stack, conventions, style guides, domain knowledge, etc. +# Example: +# context: | +# Tech stack: TypeScript, React, Node.js +# We use conventional commits +# Domain: e-commerce platform + +# Per-artifact rules (optional) +# Add custom rules for specific artifacts. +# Example: +# rules: +# proposal: +# - Keep proposals under 500 words +# - Always include a "Non-goals" section +# tasks: +# - Break tasks into chunks of max 2 hours diff --git a/openspec/specs/auth/auth-contract.md b/openspec/specs/auth/auth-contract.md new file mode 100644 index 0000000..78d39fd --- /dev/null +++ b/openspec/specs/auth/auth-contract.md @@ -0,0 +1,52 @@ +# Authentication Contract (xconfadmin) + +## Purpose +This document defines the authentication behavior provided by +xconfadmin as a standalone library. + +## Scope +This specification describes: +- credential validation +- identity resolution +- authentication success and failure outcomes +- fail-fast termination after authentication or authorization failure + +This specification does not describe: +- business-specific policy enforcement +- application-level authorization +- downstream extensions or constraints + +## Guarantees + +### Credential Validation +The system SHALL validate supplied credentials and determine +their validity deterministically. + +### Authentication Result +On successful authentication, the system SHALL return an +identity representation suitable for downstream use. + +### Failure Modes +Authentication failures SHALL result in defined error categories. +Failure handling is subject to the Fail-Fast Termination guarantee +defined below. + + +### Fail-Fast Termination + +After an authentication failure produced by this system, or +an authorization failure surfaced through this system, +request processing MUST terminate immediately. + +No downstream handler logic, middleware continuation, or +post-failure side effects SHALL occur after such a failure. + +This contract does not define authorization semantics; it specifies +fail-fast termination behavior when such failures are emitted +through this authentication boundary. + + +## Extension Notice +Downstream systems (including xconfas) may impose additional +authentication or authorization constraints beyond this contract. +Those constraints are explicitly outside the scope of this specification. \ No newline at end of file