From 2c4fe53ced9d945f68e36205b3de8b89dd28669e Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Fri, 9 Oct 2026 15:12:03 +1300 Subject: [PATCH 01/28] Authorize every gateway API route; scope API tokens to their own permissions The authenticated /api/v1 group only checked login and MFA. Admin handlers (users, API tokens, settings, sessions, audit logs) never checked roles, so any logged-in user or API token, including the CircleCI token, could create admins, mint convox:*:* tokens, or turn off deploy approvals and MFA. The checks were lost in the gin rewrite. - New Authorize middleware runs after Authenticated. It enforces the access policy declared for every route in rbac's route table and denies routes without a policy. Admin routes require their gateway permission. Self-service routes (own MFA, /info, settings reads) are open to any logged-in human. - API tokens are allowed only on the routes the CLI uses in CI: /info, /rack, creating a deploy approval request and reading one by ID. Tokens can no longer approve deploys or call any admin endpoint. - rbac.Manager.Authorize(principal, permission) replaces Enforce, EnforceUser and EnforceForAPIToken. Humans are checked against the roles on their current DB record, so demotion and deletion apply on the next request instead of after a restart (Casbin user groupings were only ever added, never removed). API tokens need the permission themselves AND their owner's current role must allow it. Suspended or locked users and their tokens are denied. - Handler and proxy checks (env reads/writes, secret masking, deploy approval view/create, Slack) now authorize the real caller. Before, an admin-owned token was treated as the admin. - Audit logs get their own gateway:audit_log:read permission instead of borrowing deploy_approval_request:read. Listing approval requests and their audit logs requires approver permission, matching the handlers. Admin role gains security:*:*, which the rack TLS cert refresh route requires. - Tokens of locked owners are rejected at authentication. Token creation honours expires_at, which the CLI already sends but the server ignored. Creating a token for an unknown user returns 404 instead of dereferencing nil. - Removed dead code: RequireRole, Authenticated's unused rbac parameter, and the header-based env permission check that nothing called (the proxy already strips Env headers). routes/authorization_test.go exercises the real router as viewer and deployer sessions, a viewer token, the production-style admin-owned cicd token and an admin-owned wildcard token, and asserts every admin endpoint refuses them. --- internal/gateway/auth/principal.go | 25 ++ internal/gateway/auth/service.go | 3 + internal/gateway/handlers/admin_tokens.go | 3 +- internal/gateway/handlers/api_env_test.go | 30 +- internal/gateway/handlers/api_handler_env.go | 10 +- internal/gateway/handlers/auth_helpers.go | 12 +- .../handlers/deploy_approval_create.go | 20 +- .../handlers/deploy_approval_helpers.go | 23 +- .../handlers/deploy_approval_notification.go | 42 ++- internal/gateway/handlers/dto.go | 9 +- .../handlers/integrations_slack_helpers.go | 7 +- .../handlers/integrations_slack_test.go | 25 +- .../gateway/handlers/testing_helpers_test.go | 25 +- internal/gateway/middleware/auth.go | 37 +-- internal/gateway/middleware/auth_test.go | 8 +- internal/gateway/middleware/authorize.go | 73 +++++ .../gateway/openapi/generated/swagger.json | 4 + internal/gateway/proxy/deny_test.go | 6 +- internal/gateway/proxy/env.go | 70 +---- internal/gateway/proxy/env_filters.go | 8 +- internal/gateway/proxy/env_test.go | 71 ++--- internal/gateway/proxy/forward.go | 4 +- internal/gateway/proxy/handler.go | 17 +- internal/gateway/proxy/handler_test.go | 15 +- internal/gateway/proxy/matrix_test.go | 16 +- internal/gateway/proxy/token_permissions.go | 3 +- internal/gateway/rbac/constants.go | 6 +- internal/gateway/rbac/gateway_routes.go | 241 ++++++++++++++++ internal/gateway/rbac/http_routes.go | 272 +++--------------- internal/gateway/rbac/interface.go | 12 +- internal/gateway/rbac/principal.go | 27 ++ internal/gateway/rbac/rbac.go | 118 ++------ internal/gateway/rbac/rbac_test.go | 242 ++++++++-------- internal/gateway/rbac/resource_string.go | 5 +- internal/gateway/rbac/roles_config.go | 1 + internal/gateway/routes/authorization_test.go | 244 ++++++++++++++++ internal/gateway/routes/main_test.go | 20 ++ internal/gateway/routes/routes.go | 3 +- web/src/lib/generated/mfa-requirements.ts | 18 +- 39 files changed, 994 insertions(+), 781 deletions(-) create mode 100644 internal/gateway/auth/principal.go create mode 100644 internal/gateway/middleware/authorize.go create mode 100644 internal/gateway/rbac/gateway_routes.go create mode 100644 internal/gateway/rbac/principal.go create mode 100644 internal/gateway/routes/authorization_test.go create mode 100644 internal/gateway/routes/main_test.go diff --git a/internal/gateway/auth/principal.go b/internal/gateway/auth/principal.go new file mode 100644 index 00000000..3f026d04 --- /dev/null +++ b/internal/gateway/auth/principal.go @@ -0,0 +1,25 @@ +package auth + +import ( + "context" + + "github.com/DocSpring/rack-gateway/internal/gateway/rbac" +) + +// Principal returns the RBAC principal for this authenticated user or API token. +func (u *User) Principal() rbac.Principal { + if u.IsAPIToken { + return rbac.TokenPrincipal(u.DBUser, u.Permissions) + } + return rbac.UserPrincipal(u.DBUser) +} + +// Authorize reports whether the authenticated caller on ctx holds the permission. +// Requests without an authenticated caller are denied. +func Authorize(ctx context.Context, manager rbac.Manager, permission string) (bool, error) { + user, ok := GetAuthUser(ctx) + if !ok || user == nil { + return false, nil + } + return manager.Authorize(user.Principal(), permission) +} diff --git a/internal/gateway/auth/service.go b/internal/gateway/auth/service.go index 6b99a111..b00296a0 100644 --- a/internal/gateway/auth/service.go +++ b/internal/gateway/auth/service.go @@ -260,6 +260,9 @@ func (a *Service) validateAPIToken(tokenString string) (*User, error) { if user.Suspended { return nil, fmt.Errorf("token owner is suspended") } + if user.LockedAt != nil { + return nil, fmt.Errorf("token owner is locked") + } userResp := &User{ Email: user.Email, diff --git a/internal/gateway/handlers/admin_tokens.go b/internal/gateway/handlers/admin_tokens.go index 253e8d3a..1fbe0588 100644 --- a/internal/gateway/handlers/admin_tokens.go +++ b/internal/gateway/handlers/admin_tokens.go @@ -75,7 +75,7 @@ func (h *AdminHandler) CreateAPIToken(c *gin.Context) { // Get user ID user, err := h.database.GetUser(targetEmail) - if err != nil { + if err != nil || user == nil { h.respondAuditError( c, http.StatusNotFound, @@ -93,6 +93,7 @@ func (h *AdminHandler) CreateAPIToken(c *gin.Context) { Name: req.Name, UserID: user.ID, Permissions: req.Permissions, + ExpiresAt: req.ExpiresAt, } if creatorEmail := strings.TrimSpace(c.GetString("user_email")); creatorEmail != "" && h.rbac != nil { if creator, err := h.rbac.GetUserWithID(creatorEmail); err == nil && creator != nil { diff --git a/internal/gateway/handlers/api_env_test.go b/internal/gateway/handlers/api_env_test.go index 7ad2dde9..684b8bc1 100644 --- a/internal/gateway/handlers/api_env_test.go +++ b/internal/gateway/handlers/api_env_test.go @@ -63,10 +63,16 @@ func newJSONContext(body []byte) (*gin.Context, *httptest.ResponseRecorder) { return c, w } -func attachUser(c *gin.Context, email, name string) { +func attachUser(t *testing.T, c *gin.Context, database *db.Database, email, name string) { + t.Helper() + dbUser, err := database.GetUser(email) + if err != nil || dbUser == nil { + t.Fatalf("load user %s: %v", email, err) + } c.Set("user_email", email) c.Set("user_name", name) - ctx := context.WithValue(c.Request.Context(), auth.UserContextKey, &auth.User{Email: email, Name: name}) + authUser := &auth.User{Email: email, Name: name, Roles: dbUser.Roles, DBUser: dbUser} + ctx := context.WithValue(c.Request.Context(), auth.UserContextKey, authUser) c.Request = c.Request.WithContext(ctx) } @@ -109,7 +115,7 @@ func TestUpdateEnvValuesSuccess(t *testing.T) { body, _ := json.Marshal(payload) c, w := newJSONContext(body) c.Params = gin.Params{{Key: "app", Value: "myapp"}} - attachUser(c, "deployer@example.com", "Deployer User") + attachUser(t, c, database, "deployer@example.com", "Deployer User") handler.UpdateEnvValues(c) @@ -171,7 +177,7 @@ func TestUpdateEnvValuesRequiresEnvSetPermission(t *testing.T) { body, _ := json.Marshal(payload) c, w := newJSONContext(body) c.Params = gin.Params{{Key: "app", Value: "myapp"}} - attachUser(c, "viewer@example.com", "Viewer User") + attachUser(t, c, database, "viewer@example.com", "Viewer User") handler.UpdateEnvValues(c) @@ -211,7 +217,7 @@ func TestUpdateEnvValuesSecretRequiresPermission(t *testing.T) { body, _ := json.Marshal(payload) c, w := newJSONContext(body) c.Params = gin.Params{{Key: "app", Value: "myapp"}} - attachUser(c, "deployer@example.com", "Deployer User") + attachUser(t, c, database, "deployer@example.com", "Deployer User") handler.UpdateEnvValues(c) @@ -255,7 +261,7 @@ func TestUpdateEnvValuesMaskedSecretWithoutExistingValueFails(t *testing.T) { body, _ := json.Marshal(payload) c, w := newJSONContext(body) c.Params = gin.Params{{Key: "app", Value: "myapp"}} - attachUser(c, "admin@example.com", "Admin User") + attachUser(t, c, database, "admin@example.com", "Admin User") handler.UpdateEnvValues(c) @@ -311,7 +317,7 @@ func TestUpdateEnvValuesProtectedKeyDenied(t *testing.T) { body, _ := json.Marshal(payload) c, w := newJSONContext(body) c.Params = gin.Params{{Key: "app", Value: "myapp"}} - attachUser(c, "admin@example.com", "Admin User") + attachUser(t, c, database, "admin@example.com", "Admin User") handler.UpdateEnvValues(c) @@ -332,7 +338,7 @@ func TestUpdateEnvValuesLogsAuditEvenWhenNoChanges(t *testing.T) { t.Fatalf("failed to seed user: %v", err) } - resp := executeUpdateEnvNoChange(t, handler) + resp := executeUpdateEnvNoChange(t, handler, database) // No release should be created when there are no changes if resp.ReleaseID != "" { @@ -356,7 +362,11 @@ func mockEnvTestServer(w http.ResponseWriter, r *http.Request) { } } -func executeUpdateEnvNoChange(t *testing.T, handler *handlers.APIHandler) handlers.UpdateEnvValuesResponse { +func executeUpdateEnvNoChange( + t *testing.T, + handler *handlers.APIHandler, + database *db.Database, +) handlers.UpdateEnvValuesResponse { t.Helper() // Set FOO to "bar" - the same value it already has payload := map[string]interface{}{ @@ -365,7 +375,7 @@ func executeUpdateEnvNoChange(t *testing.T, handler *handlers.APIHandler) handle body, _ := json.Marshal(payload) c, w := newJSONContext(body) c.Params = gin.Params{{Key: "app", Value: "myapp"}} - attachUser(c, "deployer@example.com", "Deployer User") + attachUser(t, c, database, "deployer@example.com", "Deployer User") handler.UpdateEnvValues(c) diff --git a/internal/gateway/handlers/api_handler_env.go b/internal/gateway/handlers/api_handler_env.go index c8b238a3..191b5b97 100644 --- a/internal/gateway/handlers/api_handler_env.go +++ b/internal/gateway/handlers/api_handler_env.go @@ -102,7 +102,7 @@ func (h *APIHandler) GetEnvValues(c *gin.Context) { email := c.GetString("user_email") name := c.GetString("user_name") - if ok, _ := h.rbac.Enforce(email, rbac.ScopeConvox, rbac.ResourceEnv, rbac.ActionRead); !ok { + if !callerCan(c, h.rbac, rbac.Convox(rbac.ResourceEnv, rbac.ActionRead)) { c.JSON(http.StatusForbidden, gin.H{"error": "You don't have permission to view environment variables."}) return } @@ -145,7 +145,7 @@ func (h *APIHandler) checkSecretPermissions( return false, true } - if ok, _ := h.rbac.Enforce(email, rbac.ScopeConvox, rbac.ResourceSecret, rbac.ActionRead); ok { + if callerCan(c, h.rbac, rbac.Convox(rbac.ResourceSecret, rbac.ActionRead)) { return true, true } @@ -329,7 +329,7 @@ func (h *APIHandler) prepareEnvUpdateContext(c *gin.Context) (*envUpdateContext, ctx.email = c.GetString("user_email") ctx.name = c.GetString("user_name") - if ok, _ := h.rbac.Enforce(ctx.email, rbac.ScopeConvox, rbac.ResourceEnv, rbac.ActionSet); !ok { + if !callerCan(c, h.rbac, rbac.Convox(rbac.ResourceEnv, rbac.ActionSet)) { c.JSON(http.StatusForbidden, gin.H{"error": "You don't have permission to modify environment variables."}) return nil, false } @@ -348,8 +348,8 @@ func (h *APIHandler) prepareEnvUpdateContext(c *gin.Context) (*envUpdateContext, ctx.baseEnv = baseEnv ctx.extraSecrets, ctx.protectedKeys = h.secretAndProtectedKeys(ctx.app) - ctx.allowSecrets, _ = h.rbac.Enforce(ctx.email, rbac.ScopeConvox, rbac.ResourceSecret, rbac.ActionSet) - ctx.canViewSecrets, _ = h.rbac.Enforce(ctx.email, rbac.ScopeConvox, rbac.ResourceSecret, rbac.ActionRead) + ctx.allowSecrets = callerCan(c, h.rbac, rbac.Convox(rbac.ResourceSecret, rbac.ActionSet)) + ctx.canViewSecrets = callerCan(c, h.rbac, rbac.Convox(rbac.ResourceSecret, rbac.ActionRead)) return ctx, true } diff --git a/internal/gateway/handlers/auth_helpers.go b/internal/gateway/handlers/auth_helpers.go index 267b6ba6..476da4ee 100644 --- a/internal/gateway/handlers/auth_helpers.go +++ b/internal/gateway/handlers/auth_helpers.go @@ -454,7 +454,7 @@ func requireAuth(c *gin.Context, rbacSvc rbac.Manager, resource rbac.Resource, a return "", false } - allowed, err := rbacSvc.Enforce(userEmail, rbac.ScopeGateway, resource, action) + allowed, err := auth.Authorize(c.Request.Context(), rbacSvc, rbac.Gateway(resource, action)) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to check permissions"}) return "", false @@ -466,3 +466,13 @@ func requireAuth(c *gin.Context, rbacSvc rbac.Manager, resource rbac.Resource, a return userEmail, true } + +// callerCan reports whether the authenticated caller (user or API token) on this request holds the permission. +func callerCan(c *gin.Context, rbacSvc rbac.Manager, permission string) bool { + allowed, err := auth.Authorize(c.Request.Context(), rbacSvc, permission) + if err != nil { + log.Printf("permission check failed for %s: %v", permission, err) + return false + } + return allowed +} diff --git a/internal/gateway/handlers/deploy_approval_create.go b/internal/gateway/handlers/deploy_approval_create.go index a7a33a2f..bc6495b8 100644 --- a/internal/gateway/handlers/deploy_approval_create.go +++ b/internal/gateway/handlers/deploy_approval_create.go @@ -32,7 +32,7 @@ func (h *APIHandler) CreateDeployApprovalRequest(c *gin.Context) { return } - if !h.authorizeCreateRequest(c, userEmail) { + if !h.authorizeCreateRequest(c) { return } @@ -106,7 +106,7 @@ func (h *APIHandler) GetDeployApprovalRequest(c *gin.Context) { return } - userEmail, dbUser, ok := h.authenticateUser(c) + _, dbUser, ok := h.authenticateUser(c) if !ok { return } @@ -116,7 +116,7 @@ func (h *APIHandler) GetDeployApprovalRequest(c *gin.Context) { return } - if !h.authorizeViewRequest(c, userEmail, dbUser, record) { + if !h.authorizeViewRequest(c, dbUser, record) { return } @@ -173,18 +173,8 @@ func (h *APIHandler) authenticateUser(c *gin.Context) (string, *db.User, bool) { return userEmail, dbUser, true } -func (h *APIHandler) authorizeCreateRequest(c *gin.Context, userEmail string) bool { - allowed, err := h.rbac.Enforce( - userEmail, - rbac.ScopeGateway, - rbac.ResourceDeployApprovalRequest, - rbac.ActionCreate, - ) - if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to check permissions"}) - return false - } - if !allowed { +func (h *APIHandler) authorizeCreateRequest(c *gin.Context) bool { + if !callerCan(c, h.rbac, rbac.Gateway(rbac.ResourceDeployApprovalRequest, rbac.ActionCreate)) { c.JSON(http.StatusForbidden, gin.H{ "error": "you do not have permission to request a deploy approval", }) diff --git a/internal/gateway/handlers/deploy_approval_helpers.go b/internal/gateway/handlers/deploy_approval_helpers.go index 85600e09..e79b682d 100644 --- a/internal/gateway/handlers/deploy_approval_helpers.go +++ b/internal/gateway/handlers/deploy_approval_helpers.go @@ -43,7 +43,7 @@ func resolveDeployApprovalRequestToken( return nil, errDeployApprovalRequestTargetMissing } - if err := validateTokenOwnership(rbacSvc, user, token); err != nil { + if err := validateTokenOwnership(rbacSvc, authUser, user, token); err != nil { return nil, err } @@ -100,26 +100,29 @@ func lookupByNameOrID(database *db.Database, identifier string) (*db.APIToken, e return database.GetAPITokenByName(identifier) } -// validateTokenOwnership validates that the user owns the token or has admin permissions. -func validateTokenOwnership(rbacSvc rbac.Manager, user *db.User, token *db.APIToken) error { +// validateTokenOwnership checks the caller may request an approval for the target token. +// An API token may only target itself; a user may target tokens they own, or any token if they are an approver. +func validateTokenOwnership(rbacSvc rbac.Manager, authUser *auth.User, user *db.User, token *db.APIToken) error { + if authUser != nil && authUser.IsAPIToken { + if authUser.TokenID != nil && *authUser.TokenID == token.ID { + return nil + } + return errDeployApprovalRequestForbidden + } if token.UserID == user.ID { return nil } - allowedAdmin, err := rbacSvc.Enforce( - user.Email, - rbac.ScopeGateway, - rbac.ResourceDeployApprovalRequest, - rbac.ActionApprove, + allowedAdmin, err := rbacSvc.Authorize( + rbac.UserPrincipal(user), + rbac.Gateway(rbac.ResourceDeployApprovalRequest, rbac.ActionApprove), ) if err != nil { return fmt.Errorf("failed to check admin permission: %w", err) } - if !allowedAdmin { return errDeployApprovalRequestForbidden } - return nil } diff --git a/internal/gateway/handlers/deploy_approval_notification.go b/internal/gateway/handlers/deploy_approval_notification.go index ad614a75..665b5547 100644 --- a/internal/gateway/handlers/deploy_approval_notification.go +++ b/internal/gateway/handlers/deploy_approval_notification.go @@ -9,6 +9,7 @@ import ( "github.com/gin-gonic/gin" "github.com/riverqueue/river" + "github.com/DocSpring/rack-gateway/internal/gateway/auth" "github.com/DocSpring/rack-gateway/internal/gateway/db" "github.com/DocSpring/rack-gateway/internal/gateway/github" "github.com/DocSpring/rack-gateway/internal/gateway/jobs" @@ -119,28 +120,39 @@ func (h *APIHandler) enqueueGitHubComment(c *gin.Context, owner, repo string, pr func (h *APIHandler) authorizeViewRequest( c *gin.Context, - userEmail string, dbUser *db.User, record *db.DeployApprovalRequest, ) bool { - allowedAdmin, err := h.rbac.Enforce( - userEmail, - rbac.ScopeGateway, - rbac.ResourceDeployApprovalRequest, - rbac.ActionApprove, - ) - if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to check permissions"}) + if !canViewDeployApprovalRequest(c, h.rbac, dbUser, record) { + c.JSON(http.StatusForbidden, gin.H{"error": "insufficient permissions"}) return false } + return true +} - ownsRequest := record.CreatedByUserID != nil && *record.CreatedByUserID == dbUser.ID - ownsToken := record.TargetUserID != nil && *record.TargetUserID == dbUser.ID - - if !allowedAdmin && !ownsRequest && !ownsToken { - c.JSON(http.StatusForbidden, gin.H{"error": "insufficient permissions"}) +// canViewDeployApprovalRequest allows approvers, the user who created or owns the target token, and the +// API token that created or is targeted by the request. Tokens never inherit their owner's visibility. +func canViewDeployApprovalRequest( + c *gin.Context, + rbacSvc rbac.Manager, + dbUser *db.User, + record *db.DeployApprovalRequest, +) bool { + authUser, ok := auth.GetAuthUser(c.Request.Context()) + if !ok || authUser == nil { return false } + if authUser.IsAPIToken { + tokenID := authUser.TokenID + return tokenID != nil && + (sameID(record.CreatedByAPITokenID, *tokenID) || record.TargetAPITokenID == *tokenID) + } + if callerCan(c, rbacSvc, rbac.Gateway(rbac.ResourceDeployApprovalRequest, rbac.ActionApprove)) { + return true + } + return sameID(record.CreatedByUserID, dbUser.ID) || sameID(record.TargetUserID, dbUser.ID) +} - return true +func sameID(field *int64, id int64) bool { + return field != nil && *field == id } diff --git a/internal/gateway/handlers/dto.go b/internal/gateway/handlers/dto.go index 8e54920d..994b0dfd 100644 --- a/internal/gateway/handlers/dto.go +++ b/internal/gateway/handlers/dto.go @@ -263,10 +263,11 @@ type UpdateUserNameRequest struct { // CreateAPITokenRequest represents the request body for creating a new API token. type CreateAPITokenRequest struct { - Name string `json:"name" binding:"required"` - UserEmail string `json:"user_email"` - Role string `json:"role"` // Role shortcut (viewer, ops, deployer, cicd, admin) - Permissions []string `json:"permissions"` // Explicit permissions (overrides role) + Name string `json:"name" binding:"required"` + UserEmail string `json:"user_email"` + Role string `json:"role"` // Role shortcut (viewer, ops, deployer, cicd, admin) + Permissions []string `json:"permissions"` // Explicit permissions (overrides role) + ExpiresAt *time.Time `json:"expires_at"` // Optional expiry; the token stops working after this time } // CreateAPITokenResponse represents the response body for API token creation. diff --git a/internal/gateway/handlers/integrations_slack_helpers.go b/internal/gateway/handlers/integrations_slack_helpers.go index c89516f0..8f5a64d6 100644 --- a/internal/gateway/handlers/integrations_slack_helpers.go +++ b/internal/gateway/handlers/integrations_slack_helpers.go @@ -21,12 +21,7 @@ func (h *AdminHandler) enforceIntegrationPermission(c *gin.Context, action rbac. return false } - allowed, err := h.rbac.Enforce(userEmail, rbac.ScopeGateway, rbac.ResourceIntegration, action) - if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to check permissions"}) - return false - } - if !allowed { + if !callerCan(c, h.rbac, rbac.Gateway(rbac.ResourceIntegration, action)) { if action == rbac.ActionRead { c.JSON(http.StatusForbidden, gin.H{"error": "insufficient permissions"}) } else { diff --git a/internal/gateway/handlers/integrations_slack_test.go b/internal/gateway/handlers/integrations_slack_test.go index 2c8a3319..aef61adf 100644 --- a/internal/gateway/handlers/integrations_slack_test.go +++ b/internal/gateway/handlers/integrations_slack_test.go @@ -26,30 +26,7 @@ func newDenyAllRBAC() *denyAllRBAC { return &denyAllRBAC{} } -func (_ *denyAllRBAC) Enforce( - _ string, - _ rbac.Scope, - _ rbac.Resource, - _ rbac.Action, -) (bool, error) { - return false, nil -} - -func (_ *denyAllRBAC) EnforceUser( - _ *db.User, - _ rbac.Scope, - _ rbac.Resource, - _ rbac.Action, -) (bool, error) { - return false, nil -} - -func (_ *denyAllRBAC) EnforceForAPIToken( - _ int64, - _ rbac.Scope, - _ rbac.Resource, - _ rbac.Action, -) (bool, error) { +func (_ *denyAllRBAC) Authorize(_ rbac.Principal, _ string) (bool, error) { return false, nil } diff --git a/internal/gateway/handlers/testing_helpers_test.go b/internal/gateway/handlers/testing_helpers_test.go index 3dd0dbd6..ab21e915 100644 --- a/internal/gateway/handlers/testing_helpers_test.go +++ b/internal/gateway/handlers/testing_helpers_test.go @@ -21,30 +21,7 @@ func newAllowAllRBAC(users ...*db.User) *allowAllRBAC { return &allowAllRBAC{users: m} } -func (_ *allowAllRBAC) Enforce( - _ string, - _ rbac.Scope, - _ rbac.Resource, - _ rbac.Action, -) (bool, error) { - return true, nil -} - -func (_ *allowAllRBAC) EnforceUser( - _ *db.User, - _ rbac.Scope, - _ rbac.Resource, - _ rbac.Action, -) (bool, error) { - return true, nil -} - -func (_ *allowAllRBAC) EnforceForAPIToken( - _ int64, - _ rbac.Scope, - _ rbac.Resource, - _ rbac.Action, -) (bool, error) { +func (_ *allowAllRBAC) Authorize(_ rbac.Principal, _ string) (bool, error) { return true, nil } diff --git a/internal/gateway/middleware/auth.go b/internal/gateway/middleware/auth.go index 7b6f95f7..e41dfc7d 100644 --- a/internal/gateway/middleware/auth.go +++ b/internal/gateway/middleware/auth.go @@ -12,11 +12,10 @@ import ( "github.com/gin-gonic/gin" "github.com/DocSpring/rack-gateway/internal/gateway/auth" - "github.com/DocSpring/rack-gateway/internal/gateway/rbac" ) // Authenticated enforces authentication for browser/admin API requests, supporting both session tokens and cookies. -func Authenticated(authService *auth.Service, _ rbac.Manager) gin.HandlerFunc { +func Authenticated(authService *auth.Service) gin.HandlerFunc { return func(c *gin.Context) { if authService == nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "authentication unavailable"}) @@ -158,37 +157,3 @@ func CLIOnly(authService *auth.Service) gin.HandlerFunc { } } } - -// RequireRole creates middleware that requires specific roles -func RequireRole(roles ...string) gin.HandlerFunc { - return func(c *gin.Context) { - userRoles, exists := c.Get("user_roles") - if !exists { - c.JSON(http.StatusForbidden, gin.H{"error": "no roles found"}) - c.Abort() - return - } - - userRoleList := userRoles.([]string) - - if !hasRequiredRole(userRoleList, roles) { - c.JSON(http.StatusForbidden, gin.H{"error": "insufficient permissions"}) - c.Abort() - return - } - - c.Next() - } -} - -// hasRequiredRole checks if the user has any of the required roles -func hasRequiredRole(userRoles, requiredRoles []string) bool { - for _, required := range requiredRoles { - for _, userRole := range userRoles { - if userRole == required { - return true - } - } - } - return false -} diff --git a/internal/gateway/middleware/auth_test.go b/internal/gateway/middleware/auth_test.go index 8f430ba0..d4305c49 100644 --- a/internal/gateway/middleware/auth_test.go +++ b/internal/gateway/middleware/auth_test.go @@ -10,7 +10,6 @@ import ( "github.com/DocSpring/rack-gateway/internal/gateway/auth" "github.com/DocSpring/rack-gateway/internal/gateway/db" - "github.com/DocSpring/rack-gateway/internal/gateway/rbac" "github.com/DocSpring/rack-gateway/internal/gateway/testutil/dbtest" ) @@ -22,11 +21,6 @@ func TestAuthenticatedSetsRequestContext(t *testing.T) { t.Fatalf("create user: %v", err) } - mgr, err := rbac.NewDBManager(database, "example.com") - if err != nil { - t.Fatalf("new rbac manager: %v", err) - } - sessionManager := auth.NewSessionManager(database, "test-secret", &auth.StaticTTLProvider{TTL: time.Hour}) service := auth.NewAuthService(nil, database, sessionManager) @@ -41,7 +35,7 @@ func TestAuthenticatedSetsRequestContext(t *testing.T) { } router := gin.New() - router.Use(Authenticated(service, mgr)) + router.Use(Authenticated(service)) var sawHandler bool router.GET("/me", func(c *gin.Context) { sawHandler = true diff --git a/internal/gateway/middleware/authorize.go b/internal/gateway/middleware/authorize.go new file mode 100644 index 00000000..516a899d --- /dev/null +++ b/internal/gateway/middleware/authorize.go @@ -0,0 +1,73 @@ +package middleware + +import ( + "net/http" + + "github.com/gin-gonic/gin" + + "github.com/DocSpring/rack-gateway/internal/gateway/auth" + gtwlog "github.com/DocSpring/rack-gateway/internal/gateway/logging" + "github.com/DocSpring/rack-gateway/internal/gateway/rbac" +) + +// Authorize enforces the access policy declared for each authenticated gateway route in +// rbac's route table. It must run after Authenticated. Routes without a declared policy are denied. +func Authorize(manager rbac.Manager) gin.HandlerFunc { + return func(c *gin.Context) { + spec, ok := rbac.LookupHTTPRoute(c.Request.Method, c.FullPath()) + if !ok { + gtwlog.Errorf("authz: no policy for method=%s path=%s", c.Request.Method, c.FullPath()) + abortForbidden(c, "no authorization policy for this endpoint") + return + } + + authUser, ok := auth.GetAuthUser(c.Request.Context()) + if !ok || authUser == nil { + c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "authentication required"}) + return + } + + if authUser.IsAPIToken && !spec.AllowAPIToken { + abortForbidden(c, "API tokens cannot use this endpoint") + return + } + + allowed, err := routeAllowed(manager, spec, authUser.Principal()) + if err != nil { + gtwlog.Errorf("authz: failed to check permissions for %s %s: %v", spec.Method, spec.Pattern, err) + c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"error": "failed to check permissions"}) + return + } + if !allowed { + abortForbidden(c, "insufficient permissions") + return + } + + c.Next() + } +} + +func routeAllowed(manager rbac.Manager, spec rbac.RouteSpec, principal rbac.Principal) (bool, error) { + switch spec.Access { + case rbac.AccessAuthenticated: + return true, nil + case rbac.AccessPermissions: + permissions := spec.PermissionStrings() + if len(permissions) == 0 { + return false, nil + } + for _, permission := range permissions { + allowed, err := manager.Authorize(principal, permission) + if err != nil || !allowed { + return false, err + } + } + return true, nil + default: + return false, nil + } +} + +func abortForbidden(c *gin.Context, message string) { + c.AbortWithStatusJSON(http.StatusForbidden, gin.H{"error": message}) +} diff --git a/internal/gateway/openapi/generated/swagger.json b/internal/gateway/openapi/generated/swagger.json index e1ed935f..f05f76d5 100644 --- a/internal/gateway/openapi/generated/swagger.json +++ b/internal/gateway/openapi/generated/swagger.json @@ -4410,6 +4410,10 @@ "name" ], "properties": { + "expires_at": { + "description": "Optional expiry; the token stops working after this time", + "type": "string" + }, "name": { "type": "string" }, diff --git a/internal/gateway/proxy/deny_test.go b/internal/gateway/proxy/deny_test.go index efa7efb9..35efd8a8 100644 --- a/internal/gateway/proxy/deny_test.go +++ b/internal/gateway/proxy/deny_test.go @@ -1,7 +1,6 @@ package proxy import ( - "context" "net/http" "net/http/httptest" "testing" @@ -11,7 +10,6 @@ import ( "github.com/stretchr/testify/require" "github.com/DocSpring/rack-gateway/internal/gateway/audit" - "github.com/DocSpring/rack-gateway/internal/gateway/auth" "github.com/DocSpring/rack-gateway/internal/gateway/config" "github.com/DocSpring/rack-gateway/internal/gateway/email" "github.com/DocSpring/rack-gateway/internal/gateway/rbac" @@ -57,9 +55,7 @@ func TestDeployerCannotDeleteApp(t *testing.T) { // Create request: DELETE /apps/myapp req := httptest.NewRequest(http.MethodDelete, "/apps/myapp", nil) // Inject authenticated session user into context - au := &auth.User{Email: "deployer@test.com", Name: "Deployer", IsAPIToken: false} - ctx := context.WithValue(req.Context(), auth.UserContextKey, au) - req = req.WithContext(ctx) + req = req.WithContext(requestAs(t, database, "deployer@test.com").Context()) rr := httptest.NewRecorder() h.ProxyToRack(rr, req) diff --git a/internal/gateway/proxy/env.go b/internal/gateway/proxy/env.go index 5a473c80..6ce003ab 100644 --- a/internal/gateway/proxy/env.go +++ b/internal/gateway/proxy/env.go @@ -43,70 +43,6 @@ func (h *Handler) logDeniedRBACAction( }) } -func (h *Handler) checkEnvSetPermissions(r *http.Request, email string) bool { - // Extract keys from known headers - keys := h.extractEnvKeysFromHeaders(r.Header) - if len(keys) == 0 { - // No explicit env changes detected; allow - return true - } - // Require env:set for any env changes - canEnvSet, _ := h.rbacManager.Enforce(email, rbac.ScopeConvox, rbac.ResourceEnv, rbac.ActionSet) - if !canEnvSet { - return false - } - // For secret keys, require secrets:set - canSecretsSet, _ := h.rbacManager.Enforce(email, rbac.ScopeConvox, rbac.ResourceSecret, rbac.ActionSet) - if !canSecretsSet { - for _, k := range keys { - if h.isSecretKey(k) { - return false - } - } - } - return true -} - -func (_ *Handler) extractEnvKeysFromHeaders(hdr http.Header) []string { - keys := make([]string, 0) - for name, vals := range hdr { - if !isEnvHeader(name) { - continue - } - keys = append(keys, extractKeysFromHeaderValues(vals)...) - } - return keys -} - -func isEnvHeader(name string) bool { - ln := strings.ToLower(name) - return ln == "env" || ln == "environment" || ln == "release-env" -} - -func extractKeysFromHeaderValues(vals []string) []string { - keys := make([]string, 0, len(vals)) - for _, v := range vals { - keys = append(keys, extractKeysFromEnvString(v)...) - } - return keys -} - -func extractKeysFromEnvString(envStr string) []string { - keys := make([]string, 0) - for _, line := range strings.Split(envStr, "\n") { - line = strings.TrimSpace(line) - if line == "" { - continue - } - parts := strings.SplitN(line, "=", 2) - k := strings.TrimSpace(parts[0]) - if k != "" { - keys = append(keys, k) - } - } - return keys -} - func (h *Handler) prepareReleaseCreate( r *http.Request, rack config.RackConfig, @@ -148,7 +84,7 @@ func (h *Handler) prepareReleaseCreate( return false, nil, nil } - canSecretsSet, _ := h.rbacManager.Enforce(email, rbac.ScopeConvox, rbac.ResourceSecret, rbac.ActionSet) + canSecretsSet := h.callerCan(r, rbac.ResourceSecret, rbac.ActionSet) merged, diffs, err := h.mergeEnvAndComputeDiffs(r, email, app, posted, order, baseEnv, canSecretsSet) if err != nil { return false, nil, nil @@ -212,7 +148,7 @@ func (h *Handler) validateSecretsPermissions( posted map[string]string, order []string, ) error { - canSecretsSet, _ := h.rbacManager.Enforce(email, rbac.ScopeConvox, rbac.ResourceSecret, rbac.ActionSet) + canSecretsSet := h.callerCan(r, rbac.ResourceSecret, rbac.ActionSet) if canSecretsSet { return nil } @@ -287,7 +223,7 @@ func (h *Handler) validateEnvPermissions( email, app string, order []string, ) error { - canEnvSet, _ := h.rbacManager.Enforce(email, rbac.ScopeConvox, rbac.ResourceEnv, rbac.ActionSet) + canEnvSet := h.callerCan(r, rbac.ResourceEnv, rbac.ActionSet) if canEnvSet { return nil } diff --git a/internal/gateway/proxy/env_filters.go b/internal/gateway/proxy/env_filters.go index 6fb6258c..c080c000 100644 --- a/internal/gateway/proxy/env_filters.go +++ b/internal/gateway/proxy/env_filters.go @@ -11,8 +11,8 @@ import ( "github.com/DocSpring/rack-gateway/internal/gateway/rbac" ) -func (h *Handler) filterReleaseEnvForUser(email string, body []byte, app string) []byte { - canEnvView, _ := h.rbacManager.Enforce(email, rbac.ScopeConvox, rbac.ResourceEnv, rbac.ActionRead) +func (h *Handler) filterReleaseEnvForUser(r *http.Request, body []byte, app string) []byte { + canEnvView := h.callerCan(r, rbac.ResourceEnv, rbac.ActionRead) var payload interface{} if err := json.Unmarshal(body, &payload); err != nil { @@ -194,8 +194,8 @@ func (h *Handler) isCommandApproved(app, command string) bool { // filterEnvironmentMapResponse masks secret keys in GET /apps/{app}/environment response. // The environment endpoint returns a flat JSON map: {"KEY1": "value1", "KEY2": "value2"} // This is different from the release format which has an "env" field with newline-separated values. -func (h *Handler) filterEnvironmentMapResponse(email string, body []byte, app string) []byte { - canEnvView, _ := h.rbacManager.Enforce(email, rbac.ScopeConvox, rbac.ResourceEnv, rbac.ActionRead) +func (h *Handler) filterEnvironmentMapResponse(r *http.Request, body []byte, app string) []byte { + canEnvView := h.callerCan(r, rbac.ResourceEnv, rbac.ActionRead) var envMap map[string]string if err := json.Unmarshal(body, &envMap); err != nil { diff --git a/internal/gateway/proxy/env_test.go b/internal/gateway/proxy/env_test.go index 948c44c0..a0c29924 100644 --- a/internal/gateway/proxy/env_test.go +++ b/internal/gateway/proxy/env_test.go @@ -52,8 +52,19 @@ func newProxyForEnvTest(t *testing.T) (*Handler, *db.Database, rbac.Manager) { return h, database, mgr } +// requestAs returns a request authenticated as the stored user with the given email. +func requestAs(t *testing.T, database *db.Database, userEmail string) *http.Request { + t.Helper() + user, err := database.GetUser(userEmail) + require.NoError(t, err) + require.NotNil(t, user) + authUser := &auth.User{Email: user.Email, Name: user.Name, Roles: user.Roles, DBUser: user} + req := httptest.NewRequest(http.MethodGet, "/apps/testapp/releases/R1", nil) + return req.WithContext(context.WithValue(req.Context(), auth.UserContextKey, authUser)) +} + func TestFilterReleaseEnvForUser(t *testing.T) { - h, _, mgr := newProxyForEnvTest(t) + h, database, mgr := newProxyForEnvTest(t) // Users require.NoError(t, mgr.SaveUser("admin@test.com", &rbac.UserConfig{Name: "Admin", Roles: []string{"admin"}})) require.NoError(t, mgr.SaveUser("ops@test.com", &rbac.UserConfig{Name: "Ops", Roles: []string{"ops"}})) @@ -66,13 +77,13 @@ func TestFilterReleaseEnvForUser(t *testing.T) { body := `{"id":"R1","env":"DATABASE_URL=postgres://...\nSECRET_KEY=abc\nREDIS_URL=redis://...\nPORT=3000\n"}` // Admin should still see masked secrets - out := h.filterReleaseEnvForUser("admin@test.com", []byte(body), "testapp") + out := h.filterReleaseEnvForUser(requestAs(t, database, "admin@test.com"), []byte(body), "testapp") s := string(out) require.Contains(t, s, "SECRET_KEY=********************") require.Contains(t, s, "DATABASE_URL=********************") // Ops sees masked sensitive values - out = h.filterReleaseEnvForUser("ops@test.com", []byte(body), "testapp") + out = h.filterReleaseEnvForUser(requestAs(t, database, "ops@test.com"), []byte(body), "testapp") s = string(out) require.Contains(t, s, "SECRET_KEY=********************") require.Contains(t, s, "DATABASE_URL=********************") @@ -80,18 +91,18 @@ func TestFilterReleaseEnvForUser(t *testing.T) { require.Contains(t, s, "PORT=3000") // Deployer same as ops - out = h.filterReleaseEnvForUser("deployer@test.com", []byte(body), "testapp") + out = h.filterReleaseEnvForUser(requestAs(t, database, "deployer@test.com"), []byte(body), "testapp") s = string(out) require.Contains(t, s, "SECRET_KEY=*********") } func TestFilterReleaseEnv_NoEnvViewMasksAll(t *testing.T) { - h, _, mgr := newProxyForEnvTest(t) + h, database, mgr := newProxyForEnvTest(t) require.NoError(t, mgr.SaveUser("viewer@test.com", &rbac.UserConfig{Name: "Viewer", Roles: []string{"viewer"}})) body := `{"id":"R1","env":"DATABASE_URL=postgres://...\nSECRET_KEY=abc\nREDIS_URL=redis://...\nPORT=3000\n"}` - out := h.filterReleaseEnvForUser("viewer@test.com", []byte(body), "testapp") + out := h.filterReleaseEnvForUser(requestAs(t, database, "viewer@test.com"), []byte(body), "testapp") s := string(out) // Should contain env, but all values masked require.Contains(t, s, "DATABASE_URL=********************") @@ -124,40 +135,8 @@ func TestAuditLogsForEnvChanges_MultipleRows(t *testing.T) { require.GreaterOrEqual(t, count, 2) } -func TestEnvSetPermissions(t *testing.T) { - h, _, mgr := newProxyForEnvTest(t) - // Users - require.NoError(t, mgr.SaveUser("admin@test.com", &rbac.UserConfig{Name: "Admin", Roles: []string{"admin"}})) - require.NoError( - t, - mgr.SaveUser("deployer@test.com", &rbac.UserConfig{Name: "Deployer", Roles: []string{"deployer"}}), - ) - - // Request with headers Env containing mixed keys - req := httptest.NewRequest(http.MethodPost, "/apps/app/releases", nil) - req.Header.Add("Env", strings.Join([]string{ - "PORT=3000", - "SECRET_KEY=abc", - "DATABASE_URL=postgres://...", - }, "\n")) - - // Deployer should be denied due to secret keys - ok := h.checkEnvSetPermissions(req, "deployer@test.com") - require.False(t, ok) - - // Admin allowed - ok = h.checkEnvSetPermissions(req, "admin@test.com") - require.True(t, ok) - - // Deployer with non-secret only should be allowed - req2 := httptest.NewRequest(http.MethodPost, "/apps/app/releases", nil) - req2.Header.Set("Env", "PORT=3000\nNODE_ENV=production") - ok = h.checkEnvSetPermissions(req2, "deployer@test.com") - require.True(t, ok) -} - func TestProxyBlocksReleaseCreateWithSecretSetForDeployer(t *testing.T) { - h, _, mgr := newProxyForEnvTest(t) + h, database, mgr := newProxyForEnvTest(t) require.NoError( t, mgr.SaveUser("deployer@test.com", &rbac.UserConfig{Name: "Deployer", Roles: []string{"deployer"}}), @@ -168,8 +147,7 @@ func TestProxyBlocksReleaseCreateWithSecretSetForDeployer(t *testing.T) { form.Set("env", "SECRET_KEY=abc\nPORT=3000") req := httptest.NewRequest(http.MethodPost, "/apps/app/releases", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - au := &auth.User{Email: "deployer@test.com", Name: "Deployer"} - req = req.WithContext(context.WithValue(req.Context(), auth.UserContextKey, au)) + req = req.WithContext(requestAs(t, database, "deployer@test.com").Context()) rr := httptest.NewRecorder() // Will be denied before attempting to forward (since rack URL is dummy) @@ -300,8 +278,7 @@ func TestProxyBlocksProtectedEnvChangesAndAudits(t *testing.T) { form.Set("env", "DATABASE_URL=abc\nPORT=3000") req := httptest.NewRequest(http.MethodPost, "/apps/app/releases", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - au := &auth.User{Email: "admin@test.com", Name: "Admin"} - req = req.WithContext(context.WithValue(req.Context(), auth.UserContextKey, au)) + req = req.WithContext(requestAs(t, database, "admin@test.com").Context()) rr := httptest.NewRecorder() h.ProxyToRack(rr, req) require.Equal(t, http.StatusForbidden, rr.Code) @@ -350,7 +327,7 @@ func TestEnvUnsetWithProtectedKeysFullFlow(t *testing.T) { require.NoError(t, err) // Use the new filterEnvironmentMapResponse which handles the /environment format - filteredBody := h.filterEnvironmentMapResponse("admin@test.com", envGetBody, appName) + filteredBody := h.filterEnvironmentMapResponse(requestAs(t, database, "admin@test.com"), envGetBody, appName) // Parse the filtered response var filteredEnv map[string]string @@ -405,7 +382,7 @@ func TestFilterEnvironmentEndpointResponse(t *testing.T) { envResponse := `{"ADMIN_PASSWORD":"real_secret_password","PORT":"3000","NODE_ENV":"production"}` // Use filterEnvironmentMapResponse (the fix for the /environment endpoint) - filtered := h.filterEnvironmentMapResponse("admin@test.com", []byte(envResponse), appName) + filtered := h.filterEnvironmentMapResponse(requestAs(t, database, "admin@test.com"), []byte(envResponse), appName) var result map[string]string require.NoError(t, json.Unmarshal(filtered, &result)) @@ -440,7 +417,7 @@ func TestFilterReleaseEnvMasksAppSpecificProtectedKeys(t *testing.T) { // Filter the release response for the admin user // We need to pass the app name so it can look up app-specific protected keys - filtered := h.filterReleaseEnvForUser("admin@test.com", []byte(releaseBody), "docspring") + filtered := h.filterReleaseEnvForUser(requestAs(t, database, "admin@test.com"), []byte(releaseBody), "docspring") // Parse the filtered response var result map[string]interface{} @@ -475,7 +452,7 @@ func TestFilterEnvironmentMasksSecretKeys(t *testing.T) { // These keys should be masked by default (secretNames configured in handler) envResponse := `{"DATABASE_URL":"postgres://secret@localhost","REDIS_URL":"redis://secret@localhost","PORT":"3000"}` - filtered := h.filterEnvironmentMapResponse("admin@test.com", []byte(envResponse), appName) + filtered := h.filterEnvironmentMapResponse(requestAs(t, database, "admin@test.com"), []byte(envResponse), appName) var result map[string]string require.NoError(t, json.Unmarshal(filtered, &result)) diff --git a/internal/gateway/proxy/forward.go b/internal/gateway/proxy/forward.go index 9563260b..4ee9eb19 100644 --- a/internal/gateway/proxy/forward.go +++ b/internal/gateway/proxy/forward.go @@ -88,12 +88,12 @@ func (h *Handler) processBufferedResponse( if filterRelease { app := extractAppFromPath(pth) - body = h.filterReleaseEnvForUser(authUserEmail, body, app) + body = h.filterReleaseEnvForUser(r, body, app) } if filterEnvironment { app := extractAppFromPath(pth) - body = h.filterEnvironmentMapResponse(authUserEmail, body, app) + body = h.filterEnvironmentMapResponse(r, body, app) } if shouldCapture { diff --git a/internal/gateway/proxy/handler.go b/internal/gateway/proxy/handler.go index 42e44387..e7f781ef 100644 --- a/internal/gateway/proxy/handler.go +++ b/internal/gateway/proxy/handler.go @@ -19,6 +19,7 @@ import ( "github.com/DocSpring/rack-gateway/internal/gateway/email" "github.com/DocSpring/rack-gateway/internal/gateway/envutil" "github.com/DocSpring/rack-gateway/internal/gateway/httpclient" + gtwlog "github.com/DocSpring/rack-gateway/internal/gateway/logging" "github.com/DocSpring/rack-gateway/internal/gateway/logutil" "github.com/DocSpring/rack-gateway/internal/gateway/rackcert" "github.com/DocSpring/rack-gateway/internal/gateway/rbac" @@ -354,14 +355,20 @@ func (h *Handler) checkUserPermissions( resource rbac.Resource, action rbac.Action, ) (bool, error) { - if authUser != nil && authUser.DBUser != nil { - return h.rbacManager.EnforceUser(authUser.DBUser, rbac.ScopeConvox, resource, action) + if authUser == nil { + return false, nil } - allowed, err := h.rbacManager.Enforce(authUser.Email, rbac.ScopeConvox, resource, action) + return h.rbacManager.Authorize(authUser.Principal(), rbac.Convox(resource, action)) +} + +// callerCan reports whether the authenticated caller on r holds convox::. +func (h *Handler) callerCan(r *http.Request, resource rbac.Resource, action rbac.Action) bool { + allowed, err := auth.Authorize(r.Context(), h.rbacManager, rbac.Convox(resource, action)) if err != nil { - return false, err + gtwlog.Errorf("proxy: permission check failed for %s: %v", rbac.Convox(resource, action), err) + return false } - return allowed, nil + return allowed } func (h *Handler) handlePermissionError( diff --git a/internal/gateway/proxy/handler_test.go b/internal/gateway/proxy/handler_test.go index dba803f4..49a59954 100644 --- a/internal/gateway/proxy/handler_test.go +++ b/internal/gateway/proxy/handler_test.go @@ -1,7 +1,6 @@ package proxy import ( - "context" "fmt" "net/http" "net/http/httptest" @@ -123,8 +122,8 @@ func TestAPITokenPermission_Check(t *testing.T) { mgr, err := rbac.NewDBManager(database, "example.com") require.NoError(t, err) - // Create a test user - user, err := database.CreateUser("test@example.com", "Test User", []string{"deployer"}) + // Token owner is an admin so the token's own permissions are the limiting factor + user, err := database.CreateUser("test@example.com", "Test User", []string{"admin"}) require.NoError(t, err) // Create an API token with specific permissions @@ -140,6 +139,7 @@ func TestAPITokenPermission_Check(t *testing.T) { Permissions: permissions, IsAPIToken: true, TokenID: &tokenID, + DBUser: user, } // Exact match @@ -162,9 +162,15 @@ func TestAPITokenPermission_Check(t *testing.T) { Permissions: wildcardPerms, IsAPIToken: true, TokenID: &tokenID2, + DBUser: user, } require.True(t, h.hasAPITokenPermission(u2, rbac.ResourceApp, rbac.ActionUpdate)) require.True(t, h.hasAPITokenPermission(u2, rbac.ResourceApp, rbac.ActionDelete)) + + // A token can never exceed its owner's current role + viewer := &db.User{ID: user.ID, Email: user.Email, Roles: []string{"viewer"}} + u2.DBUser = viewer + require.False(t, h.hasAPITokenPermission(u2, rbac.ResourceApp, rbac.ActionDelete)) } func TestCaptureResourceCreatorStoresMappings(t *testing.T) { @@ -260,8 +266,7 @@ func TestProxyToRackLogsReleaseAuditAndUserResource(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/apps/my-app/builds", strings.NewReader(`{"git_sha":"abc"}`)) req.Header.Set("Content-Type", "application/json") req.Header.Set("X-User-Name", "Creator") - au := &auth.User{Email: "creator@example.com", Name: "Creator"} - req = req.WithContext(context.WithValue(req.Context(), auth.UserContextKey, au)) + req = req.WithContext(requestAs(t, database, "creator@example.com").Context()) rr := httptest.NewRecorder() h.ProxyToRack(rr, req) diff --git a/internal/gateway/proxy/matrix_test.go b/internal/gateway/proxy/matrix_test.go index e936cecd..39b4a40a 100644 --- a/internal/gateway/proxy/matrix_test.go +++ b/internal/gateway/proxy/matrix_test.go @@ -57,14 +57,22 @@ func TestPermissionMatrix_DeployerVsAdmin(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { // Deployer - ok, err := mgr.Enforce("deployer@test.com", c.scope, c.resource, c.action) - require.NoError(t, err) + permission := rbac.Permission(c.scope, c.resource, c.action) + ok := userAllowed(t, mgr, database, "deployer@test.com", permission) require.Equal(t, c.deployerAllow, ok, "deployer mismatch for %s:%s:%s", c.scope, c.resource, c.action) // Admin - ok, err = mgr.Enforce("admin@test.com", c.scope, c.resource, c.action) - require.NoError(t, err) + ok = userAllowed(t, mgr, database, "admin@test.com", permission) require.Equal(t, c.adminAllow, ok, "admin mismatch for %s:%s:%s", c.scope, c.resource, c.action) }) } } + +func userAllowed(t *testing.T, mgr *rbac.DBManager, database *db.Database, email, permission string) bool { + t.Helper() + user, err := database.GetUser(email) + require.NoError(t, err) + ok, err := mgr.Authorize(rbac.UserPrincipal(user), permission) + require.NoError(t, err) + return ok +} diff --git a/internal/gateway/proxy/token_permissions.go b/internal/gateway/proxy/token_permissions.go index c34ede03..1a39aeb1 100644 --- a/internal/gateway/proxy/token_permissions.go +++ b/internal/gateway/proxy/token_permissions.go @@ -40,8 +40,7 @@ func (h *Handler) hasAPITokenPermission(authUser *auth.User, resource rbac.Resou return false } - // Use RBAC manager to check permissions, which handles deploy_with_approval logic - allowed, err := h.rbacManager.EnforceForAPIToken(*authUser.TokenID, rbac.ScopeConvox, resource, action) + allowed, err := h.rbacManager.Authorize(authUser.Principal(), rbac.Convox(resource, action)) if err != nil { // Error checking permission, deny access return false diff --git a/internal/gateway/rbac/constants.go b/internal/gateway/rbac/constants.go index 6e9cf865..9b39546b 100644 --- a/internal/gateway/rbac/constants.go +++ b/internal/gateway/rbac/constants.go @@ -141,15 +141,17 @@ const ( ResourceMFAVerification // mfa_verification // ResourceTrustedDevice identifies trusted device resources. ResourceTrustedDevice // trusted_device + // ResourceAuditLog identifies the gateway audit log. + ResourceAuditLog // audit_log ) // IsValid reports whether the resource represents a defined value. -func (r Resource) IsValid() bool { return r <= ResourceTrustedDevice } +func (r Resource) IsValid() bool { return r <= ResourceAuditLog } // ParseResource converts a string name into a Resource value. func ParseResource(v string) (Resource, error) { // Try each known value - for r := ResourceApp; r <= ResourceTrustedDevice; r++ { + for r := ResourceApp; r <= ResourceAuditLog; r++ { if r.String() == v { return r, nil } diff --git a/internal/gateway/rbac/gateway_routes.go b/internal/gateway/rbac/gateway_routes.go new file mode 100644 index 00000000..c0b194c7 --- /dev/null +++ b/internal/gateway/rbac/gateway_routes.go @@ -0,0 +1,241 @@ +package rbac + +import ( + "strings" + + "github.com/DocSpring/rack-gateway/internal/gateway/settings" +) + +// Authorization and MFA policy for every authenticated gateway (non-proxy) route. +// The Authorize middleware denies any route missing from this table. + +func routeSlug(segment string) string { + return strings.ReplaceAll(segment, "_", "-") +} + +func appSettingPath(key settings.AppSettingKey) string { + return "/api/v1/apps/:app/settings/" + routeSlug(key.String()) +} + +func appSettingsGroupPath(group settings.AppSettingGroup) string { + return "/api/v1/apps/:app/settings/" + routeSlug(string(group)) +} + +func globalSettingsGroupPath(group settings.GlobalSettingGroup) string { + return "/api/v1/settings/" + routeSlug(string(group)) +} + +func settingsActionPath(segment string) string { + return "/api/v1/settings/" + routeSlug(segment) +} + +var httpRouteSpecs = []RouteSpec{ + // MFA management + newSelfRoute("GET", "/api/v1/auth/mfa/status"), + newSelfRoute("POST", "/api/v1/auth/mfa/enroll/totp/start", Auth(ResourceMFAMethod, ActionCreate)), + newSelfRoute("POST", "/api/v1/auth/mfa/enroll/totp/confirm", Auth(ResourceMFAMethod, ActionCreate)), + newSelfRoute("POST", "/api/v1/auth/mfa/enroll/yubiotp/start", Auth(ResourceMFAMethod, ActionCreate)), + newSelfRoute("POST", "/api/v1/auth/mfa/enroll/webauthn/start", Auth(ResourceMFAMethod, ActionCreate)), + newSelfRoute("POST", "/api/v1/auth/mfa/enroll/webauthn/confirm", Auth(ResourceMFAMethod, ActionCreate)), + newSelfRoute("POST", "/api/v1/auth/mfa/verify", Auth(ResourceMFAVerification, ActionCreate)), + newSelfRoute("POST", "/api/v1/auth/mfa/webauthn/assertion/start", Auth(ResourceMFAVerification, ActionCreate)), + newSelfRoute("POST", "/api/v1/auth/mfa/webauthn/assertion/verify", Auth(ResourceMFAVerification, ActionCreate)), + newSelfRoute("PUT", "/api/v1/auth/mfa/preferred-method", Auth(ResourceMFAPreferences, ActionUpdate)), + newSelfRoute("PUT", "/api/v1/auth/mfa/methods/:methodID", Auth(ResourceMFAMethod, ActionUpdate)), + newSelfRoute("POST", "/api/v1/auth/mfa/backup-codes/regenerate", Auth(ResourceMFABackupCodes, ActionGenerate)), + newSelfRoute("POST", "/api/v1/auth/mfa/trusted-devices/trust", Auth(ResourceTrustedDevice, ActionCreate)), + newSelfRoute("DELETE", "/api/v1/auth/mfa/trusted-devices/:deviceID", Auth(ResourceTrustedDevice, ActionDelete)), + newSelfRoute("DELETE", "/api/v1/auth/mfa/methods/:methodID", Auth(ResourceMFAMethod, ActionDelete)), + + // Authenticated info + {Method: "GET", Pattern: "/api/v1/info", Access: AccessAuthenticated, AllowAPIToken: true}, + newSelfRoute("GET", "/api/v1/created-by"), + newTokenRoute("GET", "/api/v1/rack", Convox(ResourceRack, ActionRead)), + newHTTPRoute("GET", "/api/v1/deploy-approval-requests", Gateway(ResourceDeployApprovalRequest, ActionApprove)), + newTokenRoute("GET", "/api/v1/deploy-approval-requests/:id", Gateway(ResourceDeployApprovalRequest, ActionRead)), + newHTTPRoute( + "GET", + "/api/v1/deploy-approval-requests/:id/audit-logs", + Gateway(ResourceDeployApprovalRequest, ActionApprove), + ), + newTokenRoute("POST", "/api/v1/deploy-approval-requests", Gateway(ResourceDeployApprovalRequest, ActionCreate)), + newHTTPRoute( + "POST", + "/api/v1/deploy-approval-requests/:id/approve", + Gateway(ResourceDeployApprovalRequest, ActionApprove), + ), + newHTTPRoute( + "POST", + "/api/v1/deploy-approval-requests/:id/reject", + Gateway(ResourceDeployApprovalRequest, ActionApprove), + ), + newHTTPRoute( + "POST", + "/api/v1/deploy-approval-requests/:id/extend", + Gateway(ResourceDeployApprovalRequest, ActionApprove), + ), + newHTTPRoute("GET", "/api/v1/apps/:app/env", Convox(ResourceEnv, ActionRead)), + newHTTPRoute("PUT", "/api/v1/apps/:app/env", Convox(ResourceEnv, ActionSet)), + + // Web-safe Convox proxies + newHTTPRoute("GET", "/api/v1/convox/apps", Convox(ResourceApp, ActionList)), + newHTTPRoute("GET", "/api/v1/convox/apps/*path", Convox(ResourceApp, ActionRead)), + newHTTPRoute("PUT", "/api/v1/convox/apps/:app/services/:name", Convox(ResourceApp, ActionUpdate)), + newHTTPRoute( + "DELETE", + "/api/v1/convox/apps/:app/processes/:pid", + Convox(ResourceProcess, ActionTerminate), + ), + newHTTPRoute("GET", "/api/v1/convox/instances", Convox(ResourceInstance, ActionList)), + newHTTPRoute("GET", "/api/v1/convox/system/processes", Convox(ResourceRack, ActionRead)), + + // Configuration & diagnostics + newSelfRoute("GET", "/api/v1/settings"), + newHTTPRoute( + "PUT", + globalSettingsGroupPath(settings.GlobalSettingGroupMFAConfiguration), + GatewayGlobalSettingGroup(settings.GlobalSettingGroupMFAConfiguration), + ), + newHTTPRoute( + "DELETE", + globalSettingsGroupPath(settings.GlobalSettingGroupMFAConfiguration), + GatewayGlobalSettingGroup(settings.GlobalSettingGroupMFAConfiguration), + ), + newHTTPRoute( + "PUT", + globalSettingsGroupPath(settings.GlobalSettingGroupAllowDestructive), + GatewayGlobalSettingGroup(settings.GlobalSettingGroupAllowDestructive), + ), + newHTTPRoute( + "DELETE", + globalSettingsGroupPath(settings.GlobalSettingGroupAllowDestructive), + GatewayGlobalSettingGroup(settings.GlobalSettingGroupAllowDestructive), + ), + newHTTPRoute( + "PUT", + globalSettingsGroupPath(settings.GlobalSettingGroupVCSAndCIDefaults), + GatewayGlobalSettingGroup(settings.GlobalSettingGroupVCSAndCIDefaults), + ), + newHTTPRoute( + "DELETE", + globalSettingsGroupPath(settings.GlobalSettingGroupVCSAndCIDefaults), + GatewayGlobalSettingGroup(settings.GlobalSettingGroupVCSAndCIDefaults), + ), + newHTTPRoute( + "PUT", + globalSettingsGroupPath(settings.GlobalSettingGroupDeployApprovals), + GatewayGlobalSettingGroup(settings.GlobalSettingGroupDeployApprovals), + ), + newHTTPRoute( + "DELETE", + globalSettingsGroupPath(settings.GlobalSettingGroupDeployApprovals), + GatewayGlobalSettingGroup(settings.GlobalSettingGroupDeployApprovals), + ), + newHTTPRoute( + "PUT", + globalSettingsGroupPath(settings.GlobalSettingGroupSessionConfiguration), + GatewayGlobalSettingGroup(settings.GlobalSettingGroupSessionConfiguration), + ), + newHTTPRoute( + "DELETE", + globalSettingsGroupPath(settings.GlobalSettingGroupSessionConfiguration), + GatewayGlobalSettingGroup(settings.GlobalSettingGroupSessionConfiguration), + ), + newHTTPRoute("POST", settingsActionPath("rack_tls_cert/refresh"), Security(ResourceSecret, ActionUpdate)), + newHTTPRoute("POST", "/api/v1/diagnostics/sentry", Gateway(ResourceIntegration, ActionUpdate)), + + // Users & roles + newSelfRoute("GET", "/api/v1/roles"), + newHTTPRoute("GET", "/api/v1/users", Gateway(ResourceUser, ActionRead)), + newHTTPRoute("GET", "/api/v1/users/:email", Gateway(ResourceUser, ActionRead)), + newHTTPRoute("POST", "/api/v1/users", Gateway(ResourceUser, ActionCreate)), + newHTTPRoute("DELETE", "/api/v1/users/:email", Gateway(ResourceUser, ActionDelete)), + newHTTPRoute("PUT", "/api/v1/users/:email", Gateway(ResourceUser, ActionUpdate)), + newHTTPRoute("PUT", "/api/v1/users/:email/name", Gateway(ResourceUser, ActionUpdateName)), + newHTTPRoute("GET", "/api/v1/users/:email/sessions", Gateway(ResourceUser, ActionRead)), + newHTTPRoute("POST", "/api/v1/users/:email/sessions/:sessionID/revoke", Gateway(ResourceUser, ActionUpdate)), + newHTTPRoute("POST", "/api/v1/users/:email/sessions/revoke_all", Gateway(ResourceUser, ActionUpdate)), + newHTTPRoute("POST", "/api/v1/users/:email/lock", Gateway(ResourceUser, ActionUpdate)), + newHTTPRoute("POST", "/api/v1/users/:email/unlock", Gateway(ResourceUser, ActionUpdate)), + + // Audit logs + newHTTPRoute("GET", "/api/v1/audit-logs", Gateway(ResourceAuditLog, ActionRead)), + newHTTPRoute("GET", "/api/v1/audit-logs/export", Gateway(ResourceAuditLog, ActionRead)), + + // API tokens + newHTTPRoute("GET", "/api/v1/api-tokens", Gateway(ResourceAPIToken, ActionRead)), + newSelfRoute("GET", "/api/v1/api-tokens/permissions"), + newHTTPRoute("GET", "/api/v1/api-tokens/:tokenID", Gateway(ResourceAPIToken, ActionRead)), + newHTTPRoute("POST", "/api/v1/api-tokens", Gateway(ResourceAPIToken, ActionCreate)), + newHTTPRoute("PUT", "/api/v1/api-tokens/:tokenID", Gateway(ResourceAPIToken, ActionUpdate)), + newHTTPRoute("DELETE", "/api/v1/api-tokens/:tokenID", Gateway(ResourceAPIToken, ActionDelete)), + + // Background jobs + newHTTPRoute("GET", "/api/v1/jobs", Gateway(ResourceJob, ActionList)), + newHTTPRoute("GET", "/api/v1/jobs/:id", Gateway(ResourceJob, ActionRead)), + newHTTPRoute("DELETE", "/api/v1/jobs/:id", Gateway(ResourceJob, ActionDelete)), + newHTTPRoute("POST", "/api/v1/jobs/:id/retry", Gateway(ResourceJob, ActionUpdate)), + + // Integrations + newHTTPRoute("GET", "/api/v1/integrations/slack", Gateway(ResourceIntegration, ActionRead)), + newHTTPRoute("POST", "/api/v1/integrations/slack/oauth/authorize", Gateway(ResourceIntegration, ActionCreate)), + newHTTPRoute("GET", "/api/v1/integrations/slack/oauth/callback", Gateway(ResourceIntegration, ActionCreate)), + newHTTPRoute("PUT", "/api/v1/integrations/slack/channels", Gateway(ResourceIntegration, ActionUpdate)), + newHTTPRoute("PUT", "/api/v1/integrations/slack/alerts", Gateway(ResourceIntegration, ActionUpdate)), + newHTTPRoute("DELETE", "/api/v1/integrations/slack", Gateway(ResourceIntegration, ActionDelete)), + newHTTPRoute("GET", "/api/v1/integrations/slack/channels/list", Gateway(ResourceIntegration, ActionRead)), + newHTTPRoute("POST", "/api/v1/integrations/slack/test", Gateway(ResourceIntegration, ActionUpdate)), + + // App-specific settings + newSelfRoute("GET", "/api/v1/apps/:app/settings"), + newHTTPRoute( + "PUT", + appSettingsGroupPath(settings.AppSettingGroupVCSCIDeploy), + GatewayAppSettingGroup(settings.AppSettingGroupVCSCIDeploy), + ), + newHTTPRoute( + "DELETE", + appSettingsGroupPath(settings.AppSettingGroupVCSCIDeploy), + GatewayAppSettingGroup(settings.AppSettingGroupVCSCIDeploy), + ), + newHTTPRoute( + "PUT", + appSettingPath(settings.AppSettingProtectedEnvVars), + GatewayAppSetting(settings.AppSettingProtectedEnvVars), + ), + newHTTPRoute( + "DELETE", + appSettingPath(settings.AppSettingProtectedEnvVars), + GatewayAppSetting(settings.AppSettingProtectedEnvVars), + ), + newHTTPRoute( + "PUT", + appSettingPath(settings.AppSettingSecretEnvVars), + GatewayAppSetting(settings.AppSettingSecretEnvVars), + ), + newHTTPRoute( + "DELETE", + appSettingPath(settings.AppSettingSecretEnvVars), + GatewayAppSetting(settings.AppSettingSecretEnvVars), + ), + newHTTPRoute( + "PUT", + appSettingPath(settings.AppSettingApprovedDeployCommands), + GatewayAppSetting(settings.AppSettingApprovedDeployCommands), + ), + newHTTPRoute( + "DELETE", + appSettingPath(settings.AppSettingApprovedDeployCommands), + GatewayAppSetting(settings.AppSettingApprovedDeployCommands), + ), + newHTTPRoute( + "PUT", + appSettingPath(settings.AppSettingServiceImagePatterns), + GatewayAppSetting(settings.AppSettingServiceImagePatterns), + ), + newHTTPRoute( + "DELETE", + appSettingPath(settings.AppSettingServiceImagePatterns), + GatewayAppSetting(settings.AppSettingServiceImagePatterns), + ), +} diff --git a/internal/gateway/rbac/http_routes.go b/internal/gateway/rbac/http_routes.go index e4e18db0..6a0c3c49 100644 --- a/internal/gateway/rbac/http_routes.go +++ b/internal/gateway/rbac/http_routes.go @@ -3,10 +3,21 @@ package rbac import ( "strings" - "github.com/DocSpring/rack-gateway/internal/gateway/settings" "github.com/DocSpring/rack-gateway/internal/util/stringset" ) +// RouteAccess selects how an authenticated gateway route is authorized. +type RouteAccess uint8 + +const ( + // AccessPermissions requires the caller to hold every permission in RouteSpec.Permissions. + // A route with this access and no permissions is denied. + AccessPermissions RouteAccess = iota + // AccessAuthenticated allows any authenticated caller (self-service endpoints). + // Permissions on these routes only select the MFA level. + AccessAuthenticated +) + // RouteSpec defines a known Convox API route and the canonical resource/action it maps to. // Resource names are singular (app, build, release, process, log, object, rack, env). // Actions are verbs like list, get, create, update, delete, promote, read, exec, start, stop. @@ -15,10 +26,14 @@ type RouteSpec struct { Pattern string // Permissions contains explicit permission strings for this route. Rack routes set // this to the canonical convox:: permission; HTTP routes supply - // gateway/auth specific permissions or leave the slice empty for MFANone. + // the gateway/auth permissions that authorize the route and select its MFA level. Permissions []string Resource Resource Action Action + // Access selects how an HTTP route is authorized (see RouteAccess). + Access RouteAccess + // AllowAPIToken permits API tokens on an HTTP route. Without it, only human users may call it. + AllowAPIToken bool } // GetMFALevel returns the MFA level required for this route @@ -68,6 +83,7 @@ func newRackRoute(method, pattern string, resource Resource, action Action) Rout } } +// newHTTPRoute declares a gateway route for human users that requires every listed permission. func newHTTPRoute(method, pattern string, permissions ...string) RouteSpec { return RouteSpec{ Method: method, @@ -76,6 +92,21 @@ func newHTTPRoute(method, pattern string, permissions ...string) RouteSpec { } } +// newTokenRoute declares a gateway route that requires every listed permission and also accepts API tokens. +func newTokenRoute(method, pattern string, permissions ...string) RouteSpec { + spec := newHTTPRoute(method, pattern, permissions...) + spec.AllowAPIToken = true + return spec +} + +// newSelfRoute declares a self-service route open to any authenticated human user. +// mfaPermissions only select the MFA level; handlers scope the data to the caller. +func newSelfRoute(method, pattern string, mfaPermissions ...string) RouteSpec { + spec := newHTTPRoute(method, pattern, mfaPermissions...) + spec.Access = AccessAuthenticated + return spec +} + // Route specs for proxied Convox rack requests (rack-proxy endpoints and audit helpers). var rackRouteSpecs = []RouteSpec{ // Processes @@ -136,237 +167,6 @@ var rackRouteSpecs = []RouteSpec{ newRackRoute("GET", "/system/releases", ResourceRack, ActionRead), } -func routeSlug(segment string) string { - return strings.ReplaceAll(segment, "_", "-") -} - -func appSettingPath(key settings.AppSettingKey) string { - return "/api/v1/apps/:app/settings/" + routeSlug(key.String()) -} - -func appSettingsGroupPath(group settings.AppSettingGroup) string { - return "/api/v1/apps/:app/settings/" + routeSlug(string(group)) -} - -func globalSettingsGroupPath(group settings.GlobalSettingGroup) string { - return "/api/v1/settings/" + routeSlug(string(group)) -} - -func settingsActionPath(segment string) string { - return "/api/v1/settings/" + routeSlug(segment) -} - -var httpRouteSpecs = []RouteSpec{ - // MFA management - newHTTPRoute("GET", "/api/v1/auth/mfa/status"), - newHTTPRoute("POST", "/api/v1/auth/mfa/enroll/totp/start", Auth(ResourceMFAMethod, ActionCreate)), - newHTTPRoute("POST", "/api/v1/auth/mfa/enroll/totp/confirm", Auth(ResourceMFAMethod, ActionCreate)), - newHTTPRoute("POST", "/api/v1/auth/mfa/enroll/yubiotp/start", Auth(ResourceMFAMethod, ActionCreate)), - newHTTPRoute("POST", "/api/v1/auth/mfa/enroll/webauthn/start", Auth(ResourceMFAMethod, ActionCreate)), - newHTTPRoute("POST", "/api/v1/auth/mfa/enroll/webauthn/confirm", Auth(ResourceMFAMethod, ActionCreate)), - newHTTPRoute("POST", "/api/v1/auth/mfa/verify", Auth(ResourceMFAVerification, ActionCreate)), - newHTTPRoute("POST", "/api/v1/auth/mfa/webauthn/assertion/start", Auth(ResourceMFAVerification, ActionCreate)), - newHTTPRoute("POST", "/api/v1/auth/mfa/webauthn/assertion/verify", Auth(ResourceMFAVerification, ActionCreate)), - newHTTPRoute("PUT", "/api/v1/auth/mfa/preferred-method", Auth(ResourceMFAPreferences, ActionUpdate)), - newHTTPRoute("PUT", "/api/v1/auth/mfa/methods/:methodID", Auth(ResourceMFAMethod, ActionUpdate)), - newHTTPRoute("POST", "/api/v1/auth/mfa/backup-codes/regenerate", Auth(ResourceMFABackupCodes, ActionGenerate)), - newHTTPRoute("POST", "/api/v1/auth/mfa/trusted-devices/trust", Auth(ResourceTrustedDevice, ActionCreate)), - newHTTPRoute("DELETE", "/api/v1/auth/mfa/trusted-devices/:deviceID", Auth(ResourceTrustedDevice, ActionDelete)), - newHTTPRoute("DELETE", "/api/v1/auth/mfa/methods/:methodID", Auth(ResourceMFAMethod, ActionDelete)), - - // Authenticated info - newHTTPRoute("GET", "/api/v1/info"), - newHTTPRoute("GET", "/api/v1/created-by"), - newHTTPRoute("GET", "/api/v1/rack", Convox(ResourceRack, ActionRead)), - newHTTPRoute("GET", "/api/v1/deploy-approval-requests", Gateway(ResourceDeployApprovalRequest, ActionRead)), - newHTTPRoute("GET", "/api/v1/deploy-approval-requests/:id", Gateway(ResourceDeployApprovalRequest, ActionRead)), - newHTTPRoute( - "GET", - "/api/v1/deploy-approval-requests/:id/audit-logs", - Gateway(ResourceDeployApprovalRequest, ActionRead), - ), - newHTTPRoute("POST", "/api/v1/deploy-approval-requests", Gateway(ResourceDeployApprovalRequest, ActionCreate)), - newHTTPRoute( - "POST", - "/api/v1/deploy-approval-requests/:id/approve", - Gateway(ResourceDeployApprovalRequest, ActionApprove), - ), - newHTTPRoute( - "POST", - "/api/v1/deploy-approval-requests/:id/reject", - Gateway(ResourceDeployApprovalRequest, ActionApprove), - ), - newHTTPRoute( - "POST", - "/api/v1/deploy-approval-requests/:id/extend", - Gateway(ResourceDeployApprovalRequest, ActionApprove), - ), - newHTTPRoute("GET", "/api/v1/apps/:app/env", Convox(ResourceEnv, ActionRead)), - newHTTPRoute("PUT", "/api/v1/apps/:app/env", Convox(ResourceEnv, ActionSet)), - - // Web-safe Convox proxies - newHTTPRoute("GET", "/api/v1/convox/apps", Convox(ResourceApp, ActionList)), - newHTTPRoute("GET", "/api/v1/convox/apps/*path", Convox(ResourceApp, ActionRead)), - newHTTPRoute("PUT", "/api/v1/convox/apps/:app/services/:name", Convox(ResourceApp, ActionUpdate)), - newHTTPRoute( - "DELETE", - "/api/v1/convox/apps/:app/processes/:pid", - Convox(ResourceProcess, ActionTerminate), - ), - newHTTPRoute("GET", "/api/v1/convox/instances", Convox(ResourceInstance, ActionList)), - newHTTPRoute("GET", "/api/v1/convox/system/processes", Convox(ResourceRack, ActionRead)), - - // Configuration & diagnostics - newHTTPRoute("GET", "/api/v1/settings"), - newHTTPRoute( - "PUT", - globalSettingsGroupPath(settings.GlobalSettingGroupMFAConfiguration), - GatewayGlobalSettingGroup(settings.GlobalSettingGroupMFAConfiguration), - ), - newHTTPRoute( - "DELETE", - globalSettingsGroupPath(settings.GlobalSettingGroupMFAConfiguration), - GatewayGlobalSettingGroup(settings.GlobalSettingGroupMFAConfiguration), - ), - newHTTPRoute( - "PUT", - globalSettingsGroupPath(settings.GlobalSettingGroupAllowDestructive), - GatewayGlobalSettingGroup(settings.GlobalSettingGroupAllowDestructive), - ), - newHTTPRoute( - "DELETE", - globalSettingsGroupPath(settings.GlobalSettingGroupAllowDestructive), - GatewayGlobalSettingGroup(settings.GlobalSettingGroupAllowDestructive), - ), - newHTTPRoute( - "PUT", - globalSettingsGroupPath(settings.GlobalSettingGroupVCSAndCIDefaults), - GatewayGlobalSettingGroup(settings.GlobalSettingGroupVCSAndCIDefaults), - ), - newHTTPRoute( - "DELETE", - globalSettingsGroupPath(settings.GlobalSettingGroupVCSAndCIDefaults), - GatewayGlobalSettingGroup(settings.GlobalSettingGroupVCSAndCIDefaults), - ), - newHTTPRoute( - "PUT", - globalSettingsGroupPath(settings.GlobalSettingGroupDeployApprovals), - GatewayGlobalSettingGroup(settings.GlobalSettingGroupDeployApprovals), - ), - newHTTPRoute( - "DELETE", - globalSettingsGroupPath(settings.GlobalSettingGroupDeployApprovals), - GatewayGlobalSettingGroup(settings.GlobalSettingGroupDeployApprovals), - ), - newHTTPRoute( - "PUT", - globalSettingsGroupPath(settings.GlobalSettingGroupSessionConfiguration), - GatewayGlobalSettingGroup(settings.GlobalSettingGroupSessionConfiguration), - ), - newHTTPRoute( - "DELETE", - globalSettingsGroupPath(settings.GlobalSettingGroupSessionConfiguration), - GatewayGlobalSettingGroup(settings.GlobalSettingGroupSessionConfiguration), - ), - newHTTPRoute("POST", settingsActionPath("rack_tls_cert/refresh"), Security(ResourceSecret, ActionUpdate)), - newHTTPRoute("POST", "/api/v1/diagnostics/sentry", Gateway(ResourceIntegration, ActionUpdate)), - - // Users & roles - newHTTPRoute("GET", "/api/v1/roles"), - newHTTPRoute("GET", "/api/v1/users"), - newHTTPRoute("GET", "/api/v1/users/:email"), - newHTTPRoute("POST", "/api/v1/users", Gateway(ResourceUser, ActionCreate)), - newHTTPRoute("DELETE", "/api/v1/users/:email", Gateway(ResourceUser, ActionDelete)), - newHTTPRoute("PUT", "/api/v1/users/:email", Gateway(ResourceUser, ActionUpdate)), - newHTTPRoute("PUT", "/api/v1/users/:email/name", Gateway(ResourceUser, ActionUpdateName)), - newHTTPRoute("GET", "/api/v1/users/:email/sessions"), - newHTTPRoute("POST", "/api/v1/users/:email/sessions/:sessionID/revoke", Gateway(ResourceUser, ActionUpdate)), - newHTTPRoute("POST", "/api/v1/users/:email/sessions/revoke_all", Gateway(ResourceUser, ActionUpdate)), - newHTTPRoute("POST", "/api/v1/users/:email/lock", Gateway(ResourceUser, ActionUpdate)), - newHTTPRoute("POST", "/api/v1/users/:email/unlock", Gateway(ResourceUser, ActionUpdate)), - - // Audit logs - newHTTPRoute("GET", "/api/v1/audit-logs", Gateway(ResourceDeployApprovalRequest, ActionRead)), - newHTTPRoute("GET", "/api/v1/audit-logs/export", Gateway(ResourceDeployApprovalRequest, ActionRead)), - - // API tokens - newHTTPRoute("GET", "/api/v1/api-tokens", Gateway(ResourceAPIToken, ActionRead)), - newHTTPRoute("GET", "/api/v1/api-tokens/permissions"), - newHTTPRoute("GET", "/api/v1/api-tokens/:tokenID", Gateway(ResourceAPIToken, ActionRead)), - newHTTPRoute("POST", "/api/v1/api-tokens", Gateway(ResourceAPIToken, ActionCreate)), - newHTTPRoute("PUT", "/api/v1/api-tokens/:tokenID", Gateway(ResourceAPIToken, ActionUpdate)), - newHTTPRoute("DELETE", "/api/v1/api-tokens/:tokenID", Gateway(ResourceAPIToken, ActionDelete)), - - // Background jobs - newHTTPRoute("GET", "/api/v1/jobs", Gateway(ResourceJob, ActionList)), - newHTTPRoute("GET", "/api/v1/jobs/:id", Gateway(ResourceJob, ActionRead)), - newHTTPRoute("DELETE", "/api/v1/jobs/:id", Gateway(ResourceJob, ActionDelete)), - newHTTPRoute("POST", "/api/v1/jobs/:id/retry", Gateway(ResourceJob, ActionUpdate)), - - // Integrations - newHTTPRoute("GET", "/api/v1/integrations/slack", Gateway(ResourceIntegration, ActionRead)), - newHTTPRoute("POST", "/api/v1/integrations/slack/oauth/authorize", Gateway(ResourceIntegration, ActionCreate)), - newHTTPRoute("GET", "/api/v1/integrations/slack/oauth/callback", Gateway(ResourceIntegration, ActionCreate)), - newHTTPRoute("PUT", "/api/v1/integrations/slack/channels", Gateway(ResourceIntegration, ActionUpdate)), - newHTTPRoute("PUT", "/api/v1/integrations/slack/alerts", Gateway(ResourceIntegration, ActionUpdate)), - newHTTPRoute("DELETE", "/api/v1/integrations/slack", Gateway(ResourceIntegration, ActionDelete)), - newHTTPRoute("GET", "/api/v1/integrations/slack/channels/list", Gateway(ResourceIntegration, ActionRead)), - newHTTPRoute("POST", "/api/v1/integrations/slack/test", Gateway(ResourceIntegration, ActionUpdate)), - - // App-specific settings - newHTTPRoute("GET", "/api/v1/apps/:app/settings"), - newHTTPRoute( - "PUT", - appSettingsGroupPath(settings.AppSettingGroupVCSCIDeploy), - GatewayAppSettingGroup(settings.AppSettingGroupVCSCIDeploy), - ), - newHTTPRoute( - "DELETE", - appSettingsGroupPath(settings.AppSettingGroupVCSCIDeploy), - GatewayAppSettingGroup(settings.AppSettingGroupVCSCIDeploy), - ), - newHTTPRoute( - "PUT", - appSettingPath(settings.AppSettingProtectedEnvVars), - GatewayAppSetting(settings.AppSettingProtectedEnvVars), - ), - newHTTPRoute( - "DELETE", - appSettingPath(settings.AppSettingProtectedEnvVars), - GatewayAppSetting(settings.AppSettingProtectedEnvVars), - ), - newHTTPRoute( - "PUT", - appSettingPath(settings.AppSettingSecretEnvVars), - GatewayAppSetting(settings.AppSettingSecretEnvVars), - ), - newHTTPRoute( - "DELETE", - appSettingPath(settings.AppSettingSecretEnvVars), - GatewayAppSetting(settings.AppSettingSecretEnvVars), - ), - newHTTPRoute( - "PUT", - appSettingPath(settings.AppSettingApprovedDeployCommands), - GatewayAppSetting(settings.AppSettingApprovedDeployCommands), - ), - newHTTPRoute( - "DELETE", - appSettingPath(settings.AppSettingApprovedDeployCommands), - GatewayAppSetting(settings.AppSettingApprovedDeployCommands), - ), - newHTTPRoute( - "PUT", - appSettingPath(settings.AppSettingServiceImagePatterns), - GatewayAppSetting(settings.AppSettingServiceImagePatterns), - ), - newHTTPRoute( - "DELETE", - appSettingPath(settings.AppSettingServiceImagePatterns), - GatewayAppSetting(settings.AppSettingServiceImagePatterns), - ), -} - var httpRouteIndex map[string]RouteSpec func init() { @@ -381,6 +181,12 @@ func httpRouteKey(method, pattern string) string { return strings.ToUpper(method) + " " + pattern } +// LookupHTTPRoute returns the route spec declared for an authenticated gateway route. +func LookupHTTPRoute(method, pattern string) (RouteSpec, bool) { + spec, ok := httpRouteIndex[httpRouteKey(method, pattern)] + return spec, ok +} + // HTTPMFAPermissions returns the declared permissions for an authenticated gateway route. func HTTPMFAPermissions(method, pattern string) ([]string, bool) { spec, ok := httpRouteIndex[httpRouteKey(method, pattern)] diff --git a/internal/gateway/rbac/interface.go b/internal/gateway/rbac/interface.go index 687e40fc..03bce106 100644 --- a/internal/gateway/rbac/interface.go +++ b/internal/gateway/rbac/interface.go @@ -1,7 +1,5 @@ package rbac -import "github.com/DocSpring/rack-gateway/internal/gateway/db" - // UserWithID extends UserConfig with database ID type UserWithID struct { ID int64 `json:"id"` @@ -11,14 +9,8 @@ type UserWithID struct { // Manager defines the interface for RBAC operations. type Manager interface { - // Enforce checks if a user has permission to perform an action - Enforce(userEmail string, scope Scope, resource Resource, action Action) (bool, error) - - // EnforceUser checks permissions using a preloaded user record (no additional DB query) - EnforceUser(user *db.User, scope Scope, resource Resource, action Action) (bool, error) - - // EnforceForAPIToken checks if an API token has permission to perform an action - EnforceForAPIToken(tokenID int64, scope Scope, resource Resource, action Action) (bool, error) + // Authorize reports whether the principal (a user or an API token) holds the permission. + Authorize(p Principal, permission string) (bool, error) // GetAllowedDomain returns the configured domain GetAllowedDomain() string diff --git a/internal/gateway/rbac/principal.go b/internal/gateway/rbac/principal.go new file mode 100644 index 00000000..8e9d4e94 --- /dev/null +++ b/internal/gateway/rbac/principal.go @@ -0,0 +1,27 @@ +package rbac + +import "github.com/DocSpring/rack-gateway/internal/gateway/db" + +// Principal is the caller being authorized. +// +// Human users are authorized by the roles currently stored on their database record. API tokens are +// authorized by their own permission list, capped by the owner's current roles, so a token can never +// do more than the person who owns it. +type Principal struct { + // User is the human making the request, or the owner of the API token. + User *db.User + // APIToken is true when the request is authenticated with an API token. + APIToken bool + // TokenPermissions is the API token's own permission list. + TokenPermissions []string +} + +// UserPrincipal returns the principal for a human user. +func UserPrincipal(user *db.User) Principal { + return Principal{User: user} +} + +// TokenPrincipal returns the principal for an API token owned by owner. +func TokenPrincipal(owner *db.User, permissions []string) Principal { + return Principal{User: owner, APIToken: true, TokenPermissions: permissions} +} diff --git a/internal/gateway/rbac/rbac.go b/internal/gateway/rbac/rbac.go index 63f50159..e7ed83e9 100644 --- a/internal/gateway/rbac/rbac.go +++ b/internal/gateway/rbac/rbac.go @@ -15,8 +15,6 @@ import ( // Database defines the database operations needed by the RBAC manager. type Database interface { GetUser(email string) (*db.User, error) - GetAPITokenByID(id int64) (*db.APIToken, error) - HasActiveDeployApprovalForApp(tokenID int64, app string) (bool, error) ListUsers() ([]*db.User, error) CreateUser(email, name string, roles []string) (*db.User, error) UpdateUserRoles(email string, roles []string) error @@ -71,85 +69,35 @@ func NewDBManager(database *db.Database, domain string) (*DBManager, error) { domain: domain, } - // Policies are already loaded via the adapter in NewEnforcer - // Just sync users from database - if err := manager.syncUsersFromDB(); err != nil { - return nil, fmt.Errorf("failed to sync users: %w", err) - } - return manager, nil } -// syncUsersFromDB loads user-role mappings from the database -func (m *DBManager) syncUsersFromDB() error { - users, err := m.db.ListUsers() - if err != nil { - return fmt.Errorf("failed to list users: %w", err) - } - - // Add user-role mappings from database - for _, user := range users { - if user.Suspended { - continue // Skip suspended users - } - for _, role := range user.Roles { - if _, err := m.enforcer.AddGroupingPolicy(user.Email, role); err != nil { - return fmt.Errorf("failed to assign role %s to %s: %w", role, user.Email, err) - } - } - } - - return nil -} - -// Enforce checks if a user has permission to perform an action -func (m *DBManager) Enforce(userEmail string, scope Scope, resource Resource, action Action) (bool, error) { - m.mu.RLock() - defer m.mu.RUnlock() - return m.enforceWithEmailLocked(userEmail, scope, resource, action) -} - -// EnforceUser checks permissions for a preloaded user without additional database access. -func (m *DBManager) EnforceUser(user *db.User, scope Scope, resource Resource, action Action) (bool, error) { - if user == nil { +// Authorize reports whether the principal holds the permission. +// +// Human users are checked against the roles on their current database record, so role changes take +// effect on the next request. API tokens must hold the permission themselves AND their owner's +// current roles must allow it. Suspended or locked users (and tokens they own) are always denied. +func (m *DBManager) Authorize(p Principal, permission string) (bool, error) { + user := p.User + if user == nil || user.Suspended || user.LockedAt != nil { return false, nil } - if user.Suspended { + if p.APIToken && !matchesAnyPermission(p.TokenPermissions, permission) { return false, nil } - m.mu.RLock() - defer m.mu.RUnlock() - return m.enforceWithEmailLocked(user.Email, scope, resource, action) -} - -func (m *DBManager) enforceWithEmailLocked(email string, scope Scope, resource Resource, action Action) (bool, error) { - permission := Permission(scope, resource, action) - ok, err := m.enforcer.Enforce(email, permission, "*") - if err != nil { - return false, fmt.Errorf("failed to enforce: %w", err) - } - return ok, nil -} -// EnforceForAPIToken checks if an API token has permission to perform an action -func (m *DBManager) EnforceForAPIToken(tokenID int64, scope Scope, resource Resource, action Action) (bool, error) { m.mu.RLock() defer m.mu.RUnlock() - - // Get the API token - token, err := m.db.GetAPITokenByID(tokenID) - if err != nil { - return false, fmt.Errorf("failed to get API token: %w", err) - } - if token == nil { - return false, nil // Token doesn't exist + for _, role := range user.Roles { + ok, err := m.enforcer.Enforce(role, permission, "*") + if err != nil { + return false, fmt.Errorf("failed to enforce: %w", err) + } + if ok { + return true, nil + } } - - // Build permission string from enum types - permission := Permission(scope, resource, action) - - // Check if permission is directly granted (with wildcard support) - return matchesAnyPermission(token.Permissions, permission), nil + return false, nil } // matchesAnyPermission checks if the requested permission matches any in the list @@ -244,31 +192,14 @@ func (m *DBManager) GetUsers() (map[string]*UserConfig, error) { // SaveUser saves or updates a user in the database func (m *DBManager) SaveUser(email string, user *UserConfig) error { - m.mu.Lock() - defer m.mu.Unlock() - - // Check if user exists existing, err := m.db.GetUser(email) if err != nil { return fmt.Errorf("failed to check existing user: %w", err) } - if existing != nil { - if err := m.updateExistingUser(email, user, existing); err != nil { - return err - } - } else { - if err := m.createNewUser(email, user); err != nil { - return err - } + return m.updateExistingUser(email, user, existing) } - - // Resync users to update Casbin policies - if err := m.syncUsersFromDB(); err != nil { - return fmt.Errorf("failed to sync users: %w", err) - } - - return nil + return m.createNewUser(email, user) } func (m *DBManager) updateExistingUser(email string, user *UserConfig, existing *db.User) error { @@ -294,18 +225,9 @@ func (m *DBManager) createNewUser(email string, user *UserConfig) error { // DeleteUser removes a user from the database func (m *DBManager) DeleteUser(email string) error { - m.mu.Lock() - defer m.mu.Unlock() - if err := m.db.DeleteUser(email); err != nil { return fmt.Errorf("failed to delete user: %w", err) } - - // Resync users to update Casbin policies - if err := m.syncUsersFromDB(); err != nil { - return fmt.Errorf("failed to sync users: %w", err) - } - return nil } diff --git a/internal/gateway/rbac/rbac_test.go b/internal/gateway/rbac/rbac_test.go index d9b38e5d..bae32116 100644 --- a/internal/gateway/rbac/rbac_test.go +++ b/internal/gateway/rbac/rbac_test.go @@ -1,8 +1,8 @@ package rbac import ( - "sync" "testing" + "time" "github.com/stretchr/testify/require" @@ -10,168 +10,156 @@ import ( "github.com/DocSpring/rack-gateway/internal/gateway/testutil/dbtest" ) -// TestEnforceDeployerPermissions verifies deployer can update but not create/delete apps. -func TestEnforceDeployerPermissions(t *testing.T) { +func newTestManager(t *testing.T) (*DBManager, *db.Database) { + t.Helper() database := dbtest.NewDatabase(t) - - // Create users - _, err := database.CreateUser("deployer@test.com", "Deployer", []string{"deployer"}) - require.NoError(t, err) - _, err = database.CreateUser("admin@test.com", "Admin", []string{"admin"}) - require.NoError(t, err) - - // RBAC manager (DB-backed) mgr, err := NewDBManager(database, "example.com") require.NoError(t, err) + return mgr, database +} - // Deployer: denied create, allowed update, denied delete - ok, err := mgr.Enforce("deployer@test.com", ScopeConvox, ResourceApp, ActionCreate) - require.NoError(t, err) - require.False(t, ok, "deployer should NOT be allowed to create apps") - - ok, err = mgr.Enforce("deployer@test.com", ScopeConvox, ResourceApp, ActionUpdate) - require.NoError(t, err) - require.True(t, ok, "deployer should be allowed to update apps") - - ok, err = mgr.Enforce("deployer@test.com", ScopeConvox, ResourceApp, ActionDelete) - require.NoError(t, err) - require.False(t, ok, "deployer should NOT be allowed to delete apps") - - // Deploy approval permissions - ok, err = mgr.Enforce("deployer@test.com", ScopeGateway, ResourceDeployApprovalRequest, ActionCreate) +func createUser(t *testing.T, database *db.Database, email string, roles ...string) *db.User { + t.Helper() + user, err := database.CreateUser(email, email, roles) require.NoError(t, err) - require.True(t, ok, "deployer should be allowed to request deploy approval") + return user +} - ok, err = mgr.Enforce("deployer@test.com", ScopeGateway, ResourceDeployApprovalRequest, ActionApprove) +func userCan(t *testing.T, mgr *DBManager, database *db.Database, email, permission string) bool { + t.Helper() + user, err := database.GetUser(email) require.NoError(t, err) - require.False(t, ok, "deployer should NOT be allowed to approve deploy approval requests") - - // Admin: allowed delete - ok, err = mgr.Enforce("admin@test.com", ScopeConvox, ResourceApp, ActionDelete) + ok, err := mgr.Authorize(UserPrincipal(user), permission) require.NoError(t, err) - require.True(t, ok, "admin should be allowed to delete apps") - - ok, err = mgr.Enforce("admin@test.com", ScopeGateway, ResourceDeployApprovalRequest, ActionApprove) - require.NoError(t, err) - require.True(t, ok, "admin should be allowed to approve deploy approval requests") + return ok } -func TestSaveUserUpdatesDisplayName(t *testing.T) { - database := dbtest.NewDatabase(t) - _, err := database.CreateUser("user@example.com", "Old Name", []string{"viewer"}) - require.NoError(t, err) +// TestAuthorizeRolePermissions verifies role permissions, inheritance and admin wildcards. +func TestAuthorizeRolePermissions(t *testing.T) { + mgr, database := newTestManager(t) + createUser(t, database, "deployer@test.com", "deployer") + createUser(t, database, "viewer@test.com", "viewer") + createUser(t, database, "admin@test.com", "admin") - mgr, err := NewDBManager(database, "example.com") - require.NoError(t, err) + cases := []struct { + email string + permission string + want bool + }{ + {"deployer@test.com", Convox(ResourceApp, ActionCreate), false}, + {"deployer@test.com", Convox(ResourceApp, ActionUpdate), true}, + {"deployer@test.com", Convox(ResourceApp, ActionDelete), false}, + {"deployer@test.com", Convox(ResourceProcess, ActionExec), true}, // inherited from ops + {"deployer@test.com", Convox(ResourceApp, ActionList), true}, // inherited from viewer + {"deployer@test.com", Gateway(ResourceDeployApprovalRequest, ActionCreate), true}, + {"deployer@test.com", Gateway(ResourceDeployApprovalRequest, ActionApprove), false}, + {"deployer@test.com", Gateway(ResourceUser, ActionCreate), false}, + {"viewer@test.com", Gateway(ResourceAPIToken, ActionCreate), false}, + {"viewer@test.com", Gateway(ResourceAuditLog, ActionRead), false}, + {"viewer@test.com", Convox(ResourceEnv, ActionRead), false}, + {"admin@test.com", Convox(ResourceApp, ActionDelete), true}, + {"admin@test.com", Gateway(ResourceDeployApprovalRequest, ActionApprove), true}, + {"admin@test.com", Gateway(ResourceAuditLog, ActionRead), true}, + {"admin@test.com", "gateway:setting_group:mfa_configuration", true}, + {"admin@test.com", Security(ResourceSecret, ActionUpdate), true}, + } + for _, tc := range cases { + require.Equal(t, tc.want, userCan(t, mgr, database, tc.email, tc.permission), "%s %s", tc.email, tc.permission) + } +} - err = mgr.SaveUser("user@example.com", &UserConfig{Name: "New Name", Roles: []string{"viewer"}}) - require.NoError(t, err) +// TestAuthorizeUsesCurrentRoles verifies role changes and deletion take effect without a restart. +func TestAuthorizeUsesCurrentRoles(t *testing.T) { + mgr, database := newTestManager(t) + createUser(t, database, "user@test.com", "admin") + deleteApps := Convox(ResourceApp, ActionDelete) + require.True(t, userCan(t, mgr, database, "user@test.com", deleteApps)) - updated, err := database.GetUser("user@example.com") - require.NoError(t, err) - require.NotNil(t, updated) - require.Equal(t, "New Name", updated.Name) + require.NoError(t, mgr.SaveUser("user@test.com", &UserConfig{Roles: []string{"viewer"}})) + require.False(t, userCan(t, mgr, database, "user@test.com", deleteApps), "demotion must apply immediately") - err = mgr.SaveUser("user@example.com", &UserConfig{Name: " ", Roles: []string{"viewer"}}) - require.NoError(t, err) + require.NoError(t, mgr.SaveUser("user@test.com", &UserConfig{Roles: []string{"admin"}})) + require.NoError(t, mgr.DeleteUser("user@test.com")) + createUser(t, database, "user@test.com", "viewer") + require.False(t, userCan(t, mgr, database, "user@test.com", deleteApps), "re-created user must not keep old roles") +} - unchanged, err := database.GetUser("user@example.com") - require.NoError(t, err) - require.NotNil(t, unchanged) - require.Equal(t, "New Name", unchanged.Name) +// TestAuthorizeDeniesSuspendedAndLockedUsers verifies blocked users get nothing. +func TestAuthorizeDeniesSuspendedAndLockedUsers(t *testing.T) { + mgr, _ := newTestManager(t) + now := time.Now() + locked := &db.User{Email: "locked@test.com", Roles: []string{"admin"}, LockedAt: &now} + suspended := &db.User{Email: "suspended@test.com", Roles: []string{"admin"}, Suspended: true} + + for _, p := range []Principal{ + UserPrincipal(locked), + UserPrincipal(suspended), + TokenPrincipal(locked, []string{"convox:*:*"}), + UserPrincipal(nil), + } { + ok, err := mgr.Authorize(p, Convox(ResourceApp, ActionList)) + require.NoError(t, err) + require.False(t, ok) + } } -func TestAPITokenPermissions(t *testing.T) { +// TestAuthorizeAPITokens verifies tokens are limited to their own permissions AND their owner's roles. +func TestAuthorizeAPITokens(t *testing.T) { + mgr, _ := newTestManager(t) + admin := &db.User{Email: "admin@test.com", Roles: []string{"admin"}} + viewer := &db.User{Email: "viewer@test.com", Roles: []string{"viewer"}} + cases := []struct { - name string - perms []string - resource Resource - action Action - want bool + name string + owner *db.User + tokenPerms []string + permission string + want bool }{ + {"token permission granted", admin, []string{"convox:app:list"}, Convox(ResourceApp, ActionList), true}, { - name: "direct permission granted", - perms: []string{"convox:app:list"}, - resource: ResourceApp, - action: ActionList, - want: true, + "admin-owned token limited to its own permissions", admin, + []string{"convox:app:list"}, + Convox(ResourceEnv, ActionRead), false, }, { - name: "direct permission denied", - perms: []string{"convox:app:list"}, - resource: ResourceApp, - action: ActionDelete, - want: false, + "admin-owned token cannot approve deploys", admin, defaultRolePermissions["cicd"], + Gateway(ResourceDeployApprovalRequest, ActionApprove), false, }, + {"wildcard token permission", admin, []string{"convox:*:*"}, Convox(ResourceApp, ActionDelete), true}, + {"token capped by owner role", viewer, []string{"convox:*:*"}, Convox(ResourceApp, ActionDelete), false}, { - name: "wildcard permission", - perms: []string{"convox:*:*"}, - resource: ResourceApp, - action: ActionDelete, - want: true, + "token with gateway wildcard capped by owner role", viewer, + []string{"gateway:*:*"}, + Gateway(ResourceUser, ActionCreate), false, }, } - for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - mgr := newTestDBManager(tc.perms) - ok, err := mgr.EnforceForAPIToken(1, ScopeConvox, tc.resource, tc.action) + ok, err := mgr.Authorize(TokenPrincipal(tc.owner, tc.tokenPerms), tc.permission) require.NoError(t, err) require.Equal(t, tc.want, ok) }) } } -func newTestDBManager(perms []string) *DBManager { - return &DBManager{ - db: &mockDatabase{ - apiToken: &db.APIToken{ID: 1, Permissions: perms}, - }, - mu: sync.RWMutex{}, - } -} - -// mockDatabase implements the Database interface for testing. -type mockDatabase struct { - apiToken *db.APIToken - hasActiveApproval bool - approvalErr error - user *db.User - users []*db.User -} - -func (m *mockDatabase) GetAPITokenByID(_ int64) (*db.APIToken, error) { - return m.apiToken, nil -} - -func (m *mockDatabase) HasActiveDeployApproval(_ int64) (bool, error) { - return m.hasActiveApproval, m.approvalErr -} - -func (m *mockDatabase) HasActiveDeployApprovalForApp(_ int64, _ string) (bool, error) { - return m.hasActiveApproval, m.approvalErr -} - -func (m *mockDatabase) GetUser(_ string) (*db.User, error) { - return m.user, nil -} - -func (m *mockDatabase) ListUsers() ([]*db.User, error) { - return m.users, nil -} +func TestSaveUserUpdatesDisplayName(t *testing.T) { + mgr, database := newTestManager(t) + createUser(t, database, "user@example.com", "viewer") -func (_ *mockDatabase) CreateUser(_ string, _ string, _ []string) (*db.User, error) { - return nil, nil -} + err := mgr.SaveUser("user@example.com", &UserConfig{Name: "New Name", Roles: []string{"viewer"}}) + require.NoError(t, err) -func (_ *mockDatabase) UpdateUserRoles(_ string, _ []string) error { - return nil -} + updated, err := database.GetUser("user@example.com") + require.NoError(t, err) + require.NotNil(t, updated) + require.Equal(t, "New Name", updated.Name) -func (_ *mockDatabase) UpdateUserName(_ string, _ string) error { - return nil -} + err = mgr.SaveUser("user@example.com", &UserConfig{Name: " ", Roles: []string{"viewer"}}) + require.NoError(t, err) -func (_ *mockDatabase) DeleteUser(_ string) error { - return nil + unchanged, err := database.GetUser("user@example.com") + require.NoError(t, err) + require.NotNil(t, unchanged) + require.Equal(t, "New Name", unchanged.Name) } diff --git a/internal/gateway/rbac/resource_string.go b/internal/gateway/rbac/resource_string.go index bf104991..3a93ed78 100644 --- a/internal/gateway/rbac/resource_string.go +++ b/internal/gateway/rbac/resource_string.go @@ -34,11 +34,12 @@ func _() { _ = x[ResourceMFAPreferences-23] _ = x[ResourceMFAVerification-24] _ = x[ResourceTrustedDevice-25] + _ = x[ResourceAuditLog-26] } -const _Resource_name = "appbuildcertdeployenvinstancelogobjectprocessrackregistryreleaseresourceapi_tokendeploy_approval_requestintegrationjobsecretsettinguserauthmfa_backup_codesmfa_methodmfa_preferencesmfa_verificationtrusted_device" +const _Resource_name = "appbuildcertdeployenvinstancelogobjectprocessrackregistryreleaseresourceapi_tokendeploy_approval_requestintegrationjobsecretsettinguserauthmfa_backup_codesmfa_methodmfa_preferencesmfa_verificationtrusted_deviceaudit_log" -var _Resource_index = [...]uint8{0, 3, 8, 12, 18, 21, 29, 32, 38, 45, 49, 57, 64, 72, 81, 104, 115, 118, 124, 131, 135, 139, 155, 165, 180, 196, 210} +var _Resource_index = [...]uint8{0, 3, 8, 12, 18, 21, 29, 32, 38, 45, 49, 57, 64, 72, 81, 104, 115, 118, 124, 131, 135, 139, 155, 165, 180, 196, 210, 219} func (i Resource) String() string { idx := int(i) - 0 diff --git a/internal/gateway/rbac/roles_config.go b/internal/gateway/rbac/roles_config.go index 0678932e..874cea2a 100644 --- a/internal/gateway/rbac/roles_config.go +++ b/internal/gateway/rbac/roles_config.go @@ -98,6 +98,7 @@ var roleConfigs = map[string]roleConfig{ Permissions: []string{ "convox:*:*", "gateway:*:*", + "security:*:*", }, }, } diff --git a/internal/gateway/routes/authorization_test.go b/internal/gateway/routes/authorization_test.go new file mode 100644 index 00000000..228c849e --- /dev/null +++ b/internal/gateway/routes/authorization_test.go @@ -0,0 +1,244 @@ +package routes + +import ( + "bytes" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "sort" + "strings" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" + + "github.com/DocSpring/rack-gateway/internal/gateway/audit" + "github.com/DocSpring/rack-gateway/internal/gateway/auth" + "github.com/DocSpring/rack-gateway/internal/gateway/auth/mfa" + "github.com/DocSpring/rack-gateway/internal/gateway/config" + "github.com/DocSpring/rack-gateway/internal/gateway/db" + "github.com/DocSpring/rack-gateway/internal/gateway/deps" + "github.com/DocSpring/rack-gateway/internal/gateway/email" + "github.com/DocSpring/rack-gateway/internal/gateway/rbac" + "github.com/DocSpring/rack-gateway/internal/gateway/settings" + "github.com/DocSpring/rack-gateway/internal/gateway/testutil/dbtest" + "github.com/DocSpring/rack-gateway/internal/gateway/token" +) + +type authzEnv struct { + router *gin.Engine + database *db.Database + tokens *token.Service + sessions *auth.SessionManager + issued []string +} + +func newAuthzEnv(t *testing.T) *authzEnv { + t.Helper() + gin.SetMode(gin.TestMode) + database := dbtest.NewDatabase(t) + for userEmail, role := range map[string]string{ + "admin@example.com": "admin", + "deployer@example.com": "deployer", + "viewer@example.com": "viewer", + } { + _, err := database.CreateUser(userEmail, userEmail, []string{role}) + require.NoError(t, err) + } + + settingsSvc := settings.NewService(database) + sessions := auth.NewSessionManager(database, "secret", settingsSvc) + mfaSettings, err := settingsSvc.GetMFASettings() + require.NoError(t, err) + rbacMgr, err := rbac.NewDBManager(database, "example.com") + require.NoError(t, err) + tokenSvc := token.NewService(database) + mfaSvc, err := mfa.NewService(database, "Test", 24*time.Hour, 10*time.Minute, []byte("pepper"), "", "", "", "", nil) + require.NoError(t, err) + + router := gin.New() + Setup(router, &Config{Gateway: &deps.Gateway{ + Config: &config.Config{Domain: "gateway.example.com", GoogleAllowedDomain: "example.com"}, + Database: database, + RBACManager: rbacMgr, + SessionManager: sessions, + AuthService: auth.NewAuthService(tokenSvc, database, sessions), + TokenService: tokenSvc, + MFAService: mfaSvc, + MFASettings: mfaSettings, + SettingsService: settingsSvc, + AuditLogger: audit.NewLogger(database), + EmailSender: email.NoopSender{}, + }}) + return &authzEnv{router: router, database: database, tokens: tokenSvc, sessions: sessions} +} + +// credentials returns request headers that authenticate as the caller. +type credentials map[string]string + +func (e *authzEnv) apiToken(t *testing.T, ownerEmail string, permissions ...string) credentials { + t.Helper() + owner, err := e.database.GetUser(ownerEmail) + require.NoError(t, err) + resp, err := e.tokens.GenerateAPIToken(&token.APITokenRequest{ + Name: fmt.Sprintf("token-%d", len(e.issued)), UserID: owner.ID, Permissions: permissions, + }) + require.NoError(t, err) + e.issued = append(e.issued, resp.Token) + return credentials{"Authorization": "Bearer " + resp.Token} +} + +func (e *authzEnv) webSession(t *testing.T, userEmail string) credentials { + t.Helper() + user, err := e.database.GetUser(userEmail) + require.NoError(t, err) + sessionToken, _, err := e.sessions.CreateSession(user, auth.SessionMetadata{Channel: "web"}) + require.NoError(t, err) + csrf, err := e.sessions.DeriveCSRFToken(sessionToken) + require.NoError(t, err) + return credentials{"Cookie": "session_token=" + sessionToken, "X-CSRF-Token": csrf} +} + +func (e *authzEnv) do( + t *testing.T, + creds credentials, + method, path string, + body interface{}, +) *httptest.ResponseRecorder { + t.Helper() + var buf bytes.Buffer + if body != nil { + require.NoError(t, json.NewEncoder(&buf).Encode(body)) + } + req := httptest.NewRequest(method, path, &buf) + req.Host = "gateway.example.com" + req.Header.Set("Content-Type", "application/json") + for k, v := range creds { + req.Header.Set(k, v) + } + w := httptest.NewRecorder() + e.router.ServeHTTP(w, req) + return w +} + +type routeCall struct { + method string + path string + body interface{} +} + +// adminOnlyCalls are admin endpoints that must reject every non-admin user and every API token. +var adminOnlyCalls = []routeCall{ + {http.MethodGet, "/api/v1/users", nil}, + {http.MethodPost, "/api/v1/users", map[string]interface{}{ + "email": "attacker@example.com", "name": "x", "roles": []string{"admin"}, + }}, + {http.MethodPut, "/api/v1/users/admin@example.com", map[string]interface{}{"roles": []string{"viewer"}}}, + {http.MethodDelete, "/api/v1/users/admin@example.com", nil}, + {http.MethodPost, "/api/v1/users/admin@example.com/lock", map[string]interface{}{"reason": "x"}}, + {http.MethodGet, "/api/v1/users/admin@example.com/sessions", nil}, + {http.MethodGet, "/api/v1/audit-logs", nil}, + {http.MethodGet, "/api/v1/audit-logs/export", nil}, + {http.MethodGet, "/api/v1/api-tokens", nil}, + {http.MethodPost, "/api/v1/api-tokens", map[string]interface{}{ + "name": "pwn", "user_email": "admin@example.com", "permissions": []string{"convox:*:*"}, + }}, + {http.MethodPut, "/api/v1/settings/deploy-approvals", map[string]interface{}{"deploy_approvals_enabled": false}}, + {http.MethodPut, "/api/v1/settings/mfa-configuration", map[string]interface{}{"mfa_require_all_users": false}}, + {http.MethodPut, "/api/v1/apps/web/settings/protected-env-vars", []string{}}, + {http.MethodGet, "/api/v1/jobs", nil}, + {http.MethodGet, "/api/v1/deploy-approval-requests", nil}, + {http.MethodPost, "/api/v1/deploy-approval-requests/00000000-0000-0000-0000-000000000000/approve", nil}, +} + +func isAuthzDenial(w *httptest.ResponseRecorder) bool { + if w.Code != http.StatusForbidden { + return false + } + body := w.Body.String() + return strings.Contains(body, "insufficient permissions") || + strings.Contains(body, "API tokens cannot use this endpoint") +} + +func TestAdminEndpointsRejectNonAdmins(t *testing.T) { + e := newAuthzEnv(t) + callers := map[string]credentials{ + "viewer session": e.webSession(t, "viewer@example.com"), + "deployer session": e.webSession(t, "deployer@example.com"), + "viewer token": e.apiToken(t, "viewer@example.com", "convox:app:list"), + // Mirrors production: the CircleCI token is owned by an admin. + "admin-owned cicd token": e.apiToken(t, "admin@example.com", rbacRolePermissions(t, "cicd")...), + "admin-owned wildcard token": e.apiToken( + t, "admin@example.com", "convox:*:*", "gateway:*:*", "security:*:*", + ), + } + for name, creds := range callers { + for _, call := range adminOnlyCalls { + w := e.do(t, creds, call.method, call.path, call.body) + require.Truef(t, isAuthzDenial(w), "%s: %s %s => %d %s", + name, call.method, call.path, w.Code, w.Body.String()) + } + } + + admin, err := e.database.GetUser("admin@example.com") + require.NoError(t, err) + require.Equal(t, []string{"admin"}, admin.Roles, "admin must not have been demoted") + attacker, err := e.database.GetUser("attacker@example.com") + require.NoError(t, err) + require.Nil(t, attacker, "no user may have been created") +} + +func TestAdminSessionPassesAuthorization(t *testing.T) { + e := newAuthzEnv(t) + creds := e.webSession(t, "admin@example.com") + for _, call := range []routeCall{ + {http.MethodGet, "/api/v1/users", nil}, + {http.MethodGet, "/api/v1/audit-logs", nil}, + {http.MethodGet, "/api/v1/api-tokens", nil}, + } { + w := e.do(t, creds, call.method, call.path, call.body) + require.Falsef(t, isAuthzDenial(w), "%s %s => %d %s", call.method, call.path, w.Code, w.Body.String()) + } +} + +func TestCICDTokenKeepsDeployApprovalAccess(t *testing.T) { + e := newAuthzEnv(t) + creds := e.apiToken(t, "admin@example.com", rbacRolePermissions(t, "cicd")...) + for _, call := range []routeCall{ + {http.MethodGet, "/api/v1/info", nil}, + {http.MethodPost, "/api/v1/deploy-approval-requests", map[string]interface{}{}}, + {http.MethodGet, "/api/v1/deploy-approval-requests/00000000-0000-0000-0000-000000000000", nil}, + } { + w := e.do(t, creds, call.method, call.path, call.body) + require.Falsef(t, isAuthzDenial(w), "%s %s => %d %s", call.method, call.path, w.Code, w.Body.String()) + } +} + +func TestEveryGatewayRouteDeclaresAnAccessPolicy(t *testing.T) { + var tokenRoutes []string + for _, spec := range rbac.HTTPRouteSpecs() { + if spec.Access == rbac.AccessPermissions { + require.NotEmptyf(t, spec.Permissions, "%s %s requires permissions but declares none", + spec.Method, spec.Pattern) + } + if spec.AllowAPIToken { + tokenRoutes = append(tokenRoutes, spec.Method+" "+spec.Pattern) + } + } + sort.Strings(tokenRoutes) + require.Equal(t, []string{ + "GET /api/v1/deploy-approval-requests/:id", + "GET /api/v1/info", + "GET /api/v1/rack", + "POST /api/v1/deploy-approval-requests", + }, tokenRoutes, "API tokens may only reach the routes the CLI uses in CI") +} + +func rbacRolePermissions(t *testing.T, role string) []string { + t.Helper() + perms, ok := rbac.DefaultRolePermissions()[role] + require.Truef(t, ok, "unknown role %s", role) + return perms +} diff --git a/internal/gateway/routes/main_test.go b/internal/gateway/routes/main_test.go new file mode 100644 index 00000000..68c1c2b1 --- /dev/null +++ b/internal/gateway/routes/main_test.go @@ -0,0 +1,20 @@ +package routes + +import ( + "os" + "testing" + + "github.com/DocSpring/rack-gateway/internal/gateway/testutil/convoxguard" +) + +func TestMain(m *testing.M) { + cleanup, err := convoxguard.Setup() + if err != nil { + panic(err) + } + code := m.Run() + if err := cleanup(); err != nil { + panic("CRITICAL: " + err.Error()) + } + os.Exit(code) +} diff --git a/internal/gateway/routes/routes.go b/internal/gateway/routes/routes.go index e65ffd8a..54605d56 100644 --- a/internal/gateway/routes/routes.go +++ b/internal/gateway/routes/routes.go @@ -41,7 +41,8 @@ func Setup(router *gin.Engine, cfg *Config) { api.GET("/health", h.health.Health) authenticated := api.Group("") - authenticated.Use(middleware.Authenticated(cfg.AuthService, cfg.RBACManager)) + authenticated.Use(middleware.Authenticated(cfg.AuthService)) + authenticated.Use(middleware.Authorize(cfg.RBACManager)) authenticated.Use(middleware.RequireMFAEnrollmentWeb(cfg.Database, cfg.MFASettings)) authenticated.Use(middleware.EnforceMFARequirements(cfg.MFAService, cfg.Database, cfg.MFASettings)) diff --git a/web/src/lib/generated/mfa-requirements.ts b/web/src/lib/generated/mfa-requirements.ts index a657e8b1..975b5aee 100644 --- a/web/src/lib/generated/mfa-requirements.ts +++ b/web/src/lib/generated/mfa-requirements.ts @@ -147,13 +147,13 @@ export const HTTP_ROUTE_MFA_REQUIREMENTS: HttpRouteMfaRequirement[] = [ { method: "GET", pattern: "/api/v1/audit-logs", - permissions: ["gateway:deploy_approval_request:read"], + permissions: ["gateway:audit_log:read"], mfaLevel: "none", }, { method: "GET", pattern: "/api/v1/audit-logs/export", - permissions: ["gateway:deploy_approval_request:read"], + permissions: ["gateway:audit_log:read"], mfaLevel: "none", }, { @@ -195,8 +195,8 @@ export const HTTP_ROUTE_MFA_REQUIREMENTS: HttpRouteMfaRequirement[] = [ { method: "GET", pattern: "/api/v1/deploy-approval-requests", - permissions: ["gateway:deploy_approval_request:read"], - mfaLevel: "none", + permissions: ["gateway:deploy_approval_request:approve"], + mfaLevel: "always", }, { method: "GET", @@ -207,8 +207,8 @@ export const HTTP_ROUTE_MFA_REQUIREMENTS: HttpRouteMfaRequirement[] = [ { method: "GET", pattern: "/api/v1/deploy-approval-requests/:id/audit-logs", - permissions: ["gateway:deploy_approval_request:read"], - mfaLevel: "none", + permissions: ["gateway:deploy_approval_request:approve"], + mfaLevel: "always", }, { method: "GET", @@ -267,19 +267,19 @@ export const HTTP_ROUTE_MFA_REQUIREMENTS: HttpRouteMfaRequirement[] = [ { method: "GET", pattern: "/api/v1/users", - permissions: [], + permissions: ["gateway:user:read"], mfaLevel: "none", }, { method: "GET", pattern: "/api/v1/users/:email", - permissions: [], + permissions: ["gateway:user:read"], mfaLevel: "none", }, { method: "GET", pattern: "/api/v1/users/:email/sessions", - permissions: [], + permissions: ["gateway:user:read"], mfaLevel: "none", }, { From d72f7dcd4eea595ebc42412a661447485c0f1d06 Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Fri, 9 Oct 2026 15:15:28 +1300 Subject: [PATCH 02/28] Close MFA bypass via factor enrollment from a pre-MFA session A session that had only passed Google login could call the MFA enrollment endpoints (MFANone), enroll its own TOTP and be marked MFA-verified, then delete the user's real factors and mint tokens. TOTP enrollment also replaced the user's backup codes and returned fresh ones, so an attacker didn't even need to enroll. - New RequireVerifiedMFASession middleware: a session whose user must complete an MFA challenge can only reach mfa/status, mfa/verify, the WebAuthn assertion endpoints and /info until it verifies. It may also complete the challenge inline with an MFA header (used by the SPA step-up dialog). - The same rule applies on /rack-proxy, so a pending web session token can't be replayed as a Bearer credential. - Adding a factor when already enrolled requires a recent step-up; first-time enrollment is unchanged. - Enrollment no longer replaces an enrolled user's backup codes (regeneration stays a separate step-up action). - MFA factor enrollment (incl. YubiOTP), deletion and backup-code regeneration now write DB audit events. --- internal/gateway/auth/mfa/enrollment.go | 20 +- internal/gateway/auth/mfa/totp.go | 13 +- internal/gateway/auth/mfa/totp_test.go | 99 +++++ internal/gateway/auth/mfa/webauthn.go | 2 +- internal/gateway/auth/mfa/yubiotp.go | 2 +- internal/gateway/db/mfa.go | 9 + internal/gateway/handlers/auth_mfa_audit.go | 45 +++ .../gateway/handlers/auth_mfa_enrollment.go | 53 +-- .../gateway/handlers/auth_mfa_management.go | 17 + internal/gateway/middleware/mfa.go | 5 +- internal/gateway/middleware/mfa_pending.go | 124 ++++++ internal/gateway/proxy/mfa_verification.go | 27 ++ internal/gateway/routes/mfa_pending_test.go | 375 ++++++++++++++++++ internal/gateway/routes/routes.go | 1 + 14 files changed, 738 insertions(+), 54 deletions(-) create mode 100644 internal/gateway/auth/mfa/totp_test.go create mode 100644 internal/gateway/handlers/auth_mfa_audit.go create mode 100644 internal/gateway/middleware/mfa_pending.go create mode 100644 internal/gateway/routes/mfa_pending_test.go diff --git a/internal/gateway/auth/mfa/enrollment.go b/internal/gateway/auth/mfa/enrollment.go index 699500a5..2fc884d7 100644 --- a/internal/gateway/auth/mfa/enrollment.go +++ b/internal/gateway/auth/mfa/enrollment.go @@ -2,6 +2,8 @@ package mfa import ( "fmt" + + "github.com/DocSpring/rack-gateway/internal/gateway/db" ) // ensureBackupCodes generates backup codes if the user doesn't have any yet. @@ -14,15 +16,19 @@ func (s *Service) ensureBackupCodes(userID int64) ([]string, error) { if len(existing) > 0 { return nil, nil } + return s.GenerateBackupCodes(userID) +} - codes, hashes, err := s.genBackupCodes() - if err != nil { - return nil, err - } - if err := s.db.ReplaceBackupCodes(userID, hashes); err != nil { - return nil, err +// backupCodesForEnrollment returns backup codes to show while a factor is being enrolled. +// A user who is not enrolled yet gets a fresh set: their codes protect nothing until the first +// factor is confirmed, and a restarted enrollment must show codes again. An enrolled user's +// existing codes are never replaced here (regeneration is a separate step-up protected action); +// they only get codes if they have none. +func (s *Service) backupCodesForEnrollment(user *db.User) ([]string, error) { + if user.MFAEnrolled { + return s.ensureBackupCodes(user.ID) } - return codes, nil + return s.GenerateBackupCodes(user.ID) } // finalizeEnrollment confirms the method and marks user as MFA enrolled diff --git a/internal/gateway/auth/mfa/totp.go b/internal/gateway/auth/mfa/totp.go index 9202d9e9..ac17b09a 100644 --- a/internal/gateway/auth/mfa/totp.go +++ b/internal/gateway/auth/mfa/totp.go @@ -35,21 +35,10 @@ func (s *Service) StartTOTPEnrollment(user *db.User) (*StartTOTPEnrollmentResult return nil, err } - backupCodes, err := s.ensureBackupCodes(user.ID) + backupCodes, err := s.backupCodesForEnrollment(user) if err != nil { return nil, err } - // TOTP always generates backup codes on first enrollment - if backupCodes == nil { - codes, hashes, err := s.genBackupCodes() - if err != nil { - return nil, err - } - if err := s.db.ReplaceBackupCodes(user.ID, hashes); err != nil { - return nil, err - } - backupCodes = codes - } return &StartTOTPEnrollmentResult{ MethodID: method.ID, diff --git a/internal/gateway/auth/mfa/totp_test.go b/internal/gateway/auth/mfa/totp_test.go new file mode 100644 index 00000000..7c3c72f3 --- /dev/null +++ b/internal/gateway/auth/mfa/totp_test.go @@ -0,0 +1,99 @@ +package mfa + +import ( + "testing" + "time" + + "github.com/pquerna/otp/totp" + + "github.com/DocSpring/rack-gateway/internal/gateway/db" +) + +func backupCodeHashes(t *testing.T, database *db.Database, userID int64) []string { + t.Helper() + + codes, err := database.ListBackupCodes(userID) + if err != nil { + t.Fatalf("failed to list backup codes: %v", err) + } + hashes := make([]string, 0, len(codes)) + for _, code := range codes { + hashes = append(hashes, code.CodeHash) + } + return hashes +} + +func enrollTOTP(t *testing.T, svc *Service, database *db.Database, user *db.User) *db.User { + t.Helper() + + start, err := svc.StartTOTPEnrollment(user) + if err != nil { + t.Fatalf("failed to start TOTP enrollment: %v", err) + } + code, err := totp.GenerateCode(start.Secret, time.Now()) + if err != nil { + t.Fatalf("failed to generate TOTP code: %v", err) + } + if err := svc.ConfirmTOTP(user, start.MethodID, code); err != nil { + t.Fatalf("failed to confirm TOTP: %v", err) + } + enrolled, err := database.GetUser(user.Email) + if err != nil || enrolled == nil || !enrolled.MFAEnrolled { + t.Fatalf("expected user to be enrolled, err=%v", err) + } + return enrolled +} + +func TestStartTOTPEnrollment_KeepsEnrolledUsersBackupCodes(t *testing.T) { + t.Parallel() + + svc, database, user := setupMFAService(t, "keep-codes@example.com", "Keep Codes") + enrolled := enrollTOTP(t, svc, database, user) + before := backupCodeHashes(t, database, enrolled.ID) + if len(before) == 0 { + t.Fatal("expected backup codes after first enrollment") + } + + result, err := svc.StartTOTPEnrollment(enrolled) + if err != nil { + t.Fatalf("failed to start second TOTP enrollment: %v", err) + } + if len(result.BackupCodes) != 0 { + t.Fatalf("expected no backup codes for an enrolled user, got %d", len(result.BackupCodes)) + } + + after := backupCodeHashes(t, database, enrolled.ID) + if len(after) != len(before) { + t.Fatalf("backup code count changed: before=%d after=%d", len(before), len(after)) + } + for i := range before { + if before[i] != after[i] { + t.Fatal("existing backup codes were replaced by a second enrollment") + } + } +} + +func TestStartTOTPEnrollment_RestartedFirstEnrollmentIssuesFreshCodes(t *testing.T) { + t.Parallel() + + svc, database, user := setupMFAService(t, "first-enroll@example.com", "First Enroll") + + first, err := svc.StartTOTPEnrollment(user) + if err != nil { + t.Fatalf("failed to start enrollment: %v", err) + } + if len(first.BackupCodes) == 0 { + t.Fatal("expected backup codes on first enrollment") + } + + second, err := svc.StartTOTPEnrollment(user) + if err != nil { + t.Fatalf("failed to restart enrollment: %v", err) + } + if len(second.BackupCodes) == 0 { + t.Fatal("expected a restarted first enrollment to show backup codes again") + } + if len(backupCodeHashes(t, database, user.ID)) != len(second.BackupCodes) { + t.Fatal("stored backup codes should match the latest set shown to the user") + } +} diff --git a/internal/gateway/auth/mfa/webauthn.go b/internal/gateway/auth/mfa/webauthn.go index 9cbdde6d..a997fe1d 100644 --- a/internal/gateway/auth/mfa/webauthn.go +++ b/internal/gateway/auth/mfa/webauthn.go @@ -52,7 +52,7 @@ func (s *Service) StartWebAuthnEnrollment(user *db.User) (*StartWebAuthnEnrollme return nil, "", err } - backupCodes, err := s.ensureBackupCodes(user.ID) + backupCodes, err := s.backupCodesForEnrollment(user) if err != nil { return nil, "", err } diff --git a/internal/gateway/auth/mfa/yubiotp.go b/internal/gateway/auth/mfa/yubiotp.go index 6c105e86..9c84b231 100644 --- a/internal/gateway/auth/mfa/yubiotp.go +++ b/internal/gateway/auth/mfa/yubiotp.go @@ -52,7 +52,7 @@ func (s *Service) StartYubiOTPEnrollment(user *db.User, yubiOTP string) (*StartY return nil, err } - backupCodes, err := s.ensureBackupCodes(user.ID) + backupCodes, err := s.backupCodesForEnrollment(user) if err != nil { return nil, err } diff --git a/internal/gateway/db/mfa.go b/internal/gateway/db/mfa.go index 29d1231b..a9746887 100644 --- a/internal/gateway/db/mfa.go +++ b/internal/gateway/db/mfa.go @@ -38,6 +38,15 @@ func IsMFAChallengeRequired(settings *MFASettings, user *User) bool { return ShouldEnforceMFA(settings, user) } +// SessionAwaitingMFA returns true when the session belongs to a user who must +// complete an MFA challenge but has not yet done so for this session. +func SessionAwaitingMFA(settings *MFASettings, user *User, session *UserSession) bool { + if session == nil || session.MFAVerifiedAt != nil { + return false + } + return IsMFAChallengeRequired(settings, user) +} + // SetUserMFAEnrolled updates the MFA enrollment status for a user. // If enrolled is true, it also sets mfa_enforced_at if not already set. func (d *Database) SetUserMFAEnrolled(userID int64, enrolled bool) error { diff --git a/internal/gateway/handlers/auth_mfa_audit.go b/internal/gateway/handlers/auth_mfa_audit.go new file mode 100644 index 00000000..bb89c20d --- /dev/null +++ b/internal/gateway/handlers/auth_mfa_audit.go @@ -0,0 +1,45 @@ +package handlers + +import ( + "encoding/json" + "log" + + "github.com/gin-gonic/gin" + + "github.com/DocSpring/rack-gateway/internal/gateway/audit" + "github.com/DocSpring/rack-gateway/internal/gateway/db" +) + +// mfaAuditEvent describes a change to a user's MFA configuration. +type mfaAuditEvent struct { + scope string + verb string + resourceType string + resource string + details map[string]interface{} +} + +// auditMFAEvent records a change to the current user's MFA factors or backup codes in the +// database audit log. +func (h *AuthHandler) auditMFAEvent(c *gin.Context, user *db.User, event mfaAuditEvent) { + if h.database == nil || h.auditLogger == nil || user == nil { + return + } + action := audit.BuildAction(event.scope, event.verb) + details, _ := json.Marshal(event.details) + entry := &db.AuditLog{ + UserEmail: user.Email, + UserName: user.Name, + ActionType: "auth", + Action: action, + ResourceType: event.resourceType, + Resource: event.resource, + Details: string(details), + Status: "success", + IPAddress: c.ClientIP(), + UserAgent: c.Request.UserAgent(), + } + if err := h.auditLogger.LogDBEntry(entry); err != nil { + log.Printf(`{"level":"error","event":"audit_log_failed","action":%q,"error":%q}`, action, err.Error()) + } +} diff --git a/internal/gateway/handlers/auth_mfa_enrollment.go b/internal/gateway/handlers/auth_mfa_enrollment.go index f603e867..0fccddd1 100644 --- a/internal/gateway/handlers/auth_mfa_enrollment.go +++ b/internal/gateway/handlers/auth_mfa_enrollment.go @@ -98,7 +98,7 @@ func (h *AuthHandler) ConfirmTOTPEnrollment(c *gin.Context) { return } - h.logMFAEnrollmentCompletion(ctx, req.Label, "totp") + h.logMFAEnrollmentCompletion(c, ctx.userRecord, req.Label, "totp") response := VerifyMFAResponse{ MFAVerifiedAt: now, @@ -146,6 +146,7 @@ func (h *AuthHandler) StartYubiOTPEnrollment(c *gin.Context) { return } + h.logMFAEnrollmentCompletion(c, ctx.userRecord, "", "yubiotp") c.JSON(http.StatusOK, result) } @@ -275,7 +276,7 @@ func (h *AuthHandler) ConfirmWebAuthnEnrollment(c *gin.Context) { return } - h.logMFAEnrollmentCompletion(ctx, label, "webauthn") + h.logMFAEnrollmentCompletion(c, ctx.userRecord, label, "webauthn") c.JSON(http.StatusOK, gin.H{"status": "enrolled", "method_id": methodID}) } @@ -299,38 +300,28 @@ func (h *AuthHandler) updateMFAMethodLabel(methodID int64, label, defaultLabel s } } -func (h *AuthHandler) logMFAEnrollmentCompletion(ctx *mfaContext, label, resourceType string) { - if h.database == nil { - return - } +func (h *AuthHandler) logMFAEnrollmentCompletion(c *gin.Context, user *db.User, label, methodType string) { methodLabel := strings.TrimSpace(label) if methodLabel == "" { - if resourceType == "totp" { - methodLabel = "Authenticator App" - } else { - methodLabel = "Security Key" - } - } - details, _ := json.Marshal(map[string]interface{}{ - "label": methodLabel, + methodLabel = defaultMFAMethodLabel(methodType) + } + h.auditMFAEvent(c, user, mfaAuditEvent{ + scope: audit.ActionScopeMFAMethod, + verb: audit.ActionVerbEnroll, + resourceType: "mfa_method", + resource: methodType, + details: map[string]interface{}{"label": methodLabel}, }) - if err := h.auditLogger.LogDBEntry(&db.AuditLog{ - UserEmail: ctx.userRecord.Email, - UserName: ctx.userRecord.Name, - ActionType: "auth", - Action: audit.BuildAction(audit.ActionScopeMFAMethod, audit.ActionVerbEnroll), - ResourceType: "mfa_method", - Resource: resourceType, - Details: string(details), - Status: "success", - IPAddress: ctx.ipAddress, - UserAgent: ctx.userAgent, - }); err != nil { - log.Printf( - `{"level":"error","event":"audit_log_failed",`+ - `"action":audit.BuildAction(audit.ActionScopeMFAMethod, audit.ActionVerbEnroll),"error":%q}`, - err, - ) +} + +func defaultMFAMethodLabel(methodType string) string { + switch methodType { + case "totp": + return "Authenticator App" + case "yubiotp": + return "Yubikey" + default: + return "Security Key" } } diff --git a/internal/gateway/handlers/auth_mfa_management.go b/internal/gateway/handlers/auth_mfa_management.go index 335d7b45..468c0d8c 100644 --- a/internal/gateway/handlers/auth_mfa_management.go +++ b/internal/gateway/handlers/auth_mfa_management.go @@ -3,13 +3,16 @@ package handlers import ( "log" "net/http" + "strconv" "time" "github.com/gin-gonic/gin" + "github.com/DocSpring/rack-gateway/internal/gateway/audit" "github.com/DocSpring/rack-gateway/internal/gateway/auth" "github.com/DocSpring/rack-gateway/internal/gateway/db" gtwlog "github.com/DocSpring/rack-gateway/internal/gateway/logging" + "github.com/DocSpring/rack-gateway/internal/gateway/rbac" ) // RegenerateBackupCodes godoc @@ -35,6 +38,13 @@ func (h *AuthHandler) RegenerateBackupCodes(c *gin.Context) { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } + h.auditMFAEvent(c, ctx.userRecord, mfaAuditEvent{ + scope: audit.ActionScopeMFABackupCodes, + verb: rbac.ActionGenerate.String(), + resourceType: "mfa_backup_codes", + resource: strconv.FormatInt(ctx.userRecord.ID, 10), + details: map[string]interface{}{"count": len(codes)}, + }) c.JSON(http.StatusOK, BackupCodesResponse{BackupCodes: codes}) } @@ -155,6 +165,13 @@ func (h *AuthHandler) DeleteMFAMethod(c *gin.Context) { c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to delete mfa method"}) return } + h.auditMFAEvent(c, userCtx.userRecord, mfaAuditEvent{ + scope: audit.ActionScopeMFAMethod, + verb: rbac.ActionDelete.String(), + resourceType: "mfa_method", + resource: strconv.FormatInt(method.ID, 10), + details: map[string]interface{}{"type": method.Type, "label": method.Label}, + }) h.handleMFADisablement(userCtx.userRecord.ID) c.JSON(http.StatusOK, StatusResponse{Status: "deleted"}) diff --git a/internal/gateway/middleware/mfa.go b/internal/gateway/middleware/mfa.go index 4c01f90a..c72f7975 100644 --- a/internal/gateway/middleware/mfa.go +++ b/internal/gateway/middleware/mfa.go @@ -79,8 +79,9 @@ func enforceAlwaysMFA(c *gin.Context, mfaService MFAVerifier, database *db.Datab denyMFA(c) return } - // API tokens don't have MFA - if authUser.IsAPIToken { + // API tokens don't have MFA. A request whose inline MFA just completed the session's + // pending MFA challenge has already proven a factor. + if authUser.IsAPIToken || c.GetBool(pendingMFAInlineVerifiedKey) { c.Next() return } diff --git a/internal/gateway/middleware/mfa_pending.go b/internal/gateway/middleware/mfa_pending.go new file mode 100644 index 00000000..90260a46 --- /dev/null +++ b/internal/gateway/middleware/mfa_pending.go @@ -0,0 +1,124 @@ +package middleware + +import ( + "net/http" + "time" + + "github.com/gin-gonic/gin" + + "github.com/DocSpring/rack-gateway/internal/gateway/auth" + "github.com/DocSpring/rack-gateway/internal/gateway/db" +) + +// pendingMFAInlineVerifiedKey marks a request whose inline MFA credential already completed the +// session's MFA challenge, so later MFA checks in the same request don't verify it a second time +// (which would trip TOTP replay protection). +const pendingMFAInlineVerifiedKey = "mfa_pending_inline_verified" + +// pendingMFAAllowedRoutes are the only routes a session may reach before it completes its MFA challenge. +var pendingMFAAllowedRoutes = map[string]struct{}{ + "GET /api/v1/auth/mfa/status": {}, + "POST /api/v1/auth/mfa/verify": {}, + "POST /api/v1/auth/mfa/webauthn/assertion/start": {}, + "POST /api/v1/auth/mfa/webauthn/assertion/verify": {}, + "GET /api/v1/info": {}, +} + +// mfaFactorEnrollmentRoutes add a new MFA factor to the current user's account. +var mfaFactorEnrollmentRoutes = map[string]struct{}{ + "POST /api/v1/auth/mfa/enroll/totp/start": {}, + "POST /api/v1/auth/mfa/enroll/totp/confirm": {}, + "POST /api/v1/auth/mfa/enroll/yubiotp/start": {}, + "POST /api/v1/auth/mfa/enroll/webauthn/start": {}, + "POST /api/v1/auth/mfa/enroll/webauthn/confirm": {}, +} + +// RequireVerifiedMFASession blocks sessions that have not completed their MFA challenge from +// everything except the challenge endpoints, and requires an already-enrolled user to pass a +// recent MFA step-up before adding another factor. Without this, a session that has only passed +// Google login could enroll its own factor and use it to satisfy MFA. +func RequireVerifiedMFASession( + mfaService MFAVerifier, + database *db.Database, + settings *db.MFASettings, +) gin.HandlerFunc { + return func(c *gin.Context) { + authUser, ok := auth.GetAuthUser(c.Request.Context()) + if !ok || authUser == nil || authUser.IsAPIToken || c.Request.Method == http.MethodOptions { + c.Next() + return + } + + route := c.Request.Method + " " + c.FullPath() + user := getUserRecord(c, database, authUser.Email) + + if db.SessionAwaitingMFA(settings, user, authUser.Session) && + !allowPendingMFARequest(c, route, mfaService, database, user, authUser) { + return + } + + if _, adding := mfaFactorEnrollmentRoutes[route]; adding && user != nil && user.MFAEnrolled { + if !checkStepUpMFA(c, mfaService, database, settings) { + return + } + } + + c.Next() + } +} + +// allowPendingMFARequest lets a session that still owes its MFA challenge reach the challenge +// endpoints, or complete the challenge inline with an MFA header. Otherwise it denies the request. +func allowPendingMFARequest( + c *gin.Context, + route string, + mfaService MFAVerifier, + database *db.Database, + user *db.User, + authUser *auth.User, +) bool { + if _, ok := pendingMFAAllowedRoutes[route]; ok { + return true + } + if completePendingMFAInline(c, mfaService, database, user, authUser) { + return true + } + if !c.IsAborted() { + denyStepUp(c) + } + return false +} + +// completePendingMFAInline verifies an inline MFA credential for a session that has not yet +// completed its MFA challenge, and marks the session as MFA-verified on success. +func completePendingMFAInline( + c *gin.Context, + mfaService MFAVerifier, + database *db.Database, + user *db.User, + authUser *auth.User, +) bool { + if mfaService == nil || database == nil || authUser.MFAType == "" || authUser.MFAValue == "" { + return false + } + if !verifyByType(c, mfaService, database, user, authUser) { + return false + } + + session := authUser.Session + now := time.Now() + if err := database.UpdateSessionMFAVerified(session.ID, now, session.TrustedDeviceID); err != nil { + c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{ + "error": "mfa_verification_record_failed", + "message": "Failed to record MFA verification. Please try again.", + }) + return false + } + session.MFAVerifiedAt = &now + + if !updateStepUpAfterSuccess(c, database, authUser) { + return false + } + c.Set(pendingMFAInlineVerifiedKey, true) + return true +} diff --git a/internal/gateway/proxy/mfa_verification.go b/internal/gateway/proxy/mfa_verification.go index 224a2240..24c04f68 100644 --- a/internal/gateway/proxy/mfa_verification.go +++ b/internal/gateway/proxy/mfa_verification.go @@ -28,6 +28,17 @@ func (h *Handler) verifyMFAIfRequired( rackConfig *config.RackConfig, start time.Time, ) error { + if h.sessionAwaitingMFA(authUser) { + w.Header().Set("X-MFA-Required", "true") + h.logMFADenial( + r, w, authUser, rackConfig, start, + "Multi-factor authentication must be completed for this session", + http.StatusUnauthorized, + fmt.Errorf("session has not completed MFA"), + ) + return fmt.Errorf("session has not completed MFA") + } + if h.mfaService == nil || h.sessionManager == nil { return nil } @@ -52,6 +63,22 @@ func (h *Handler) verifyMFAIfRequired( return h.checkSessionStepUp(r, w, authUser, rackConfig, mfaLevel, start) } +// sessionAwaitingMFA reports whether a session token is being used before its MFA challenge +// was completed (e.g. a web session cookie value replayed as a Bearer token on the CLI proxy). +// Missing MFA settings are treated as enforced, so this fails closed. +func (h *Handler) sessionAwaitingMFA(authUser *auth.User) bool { + if authUser == nil || authUser.IsAPIToken || authUser.Session == nil { + return false + } + var settings *db.MFASettings + if h.settingsService != nil { + if loaded, err := h.settingsService.GetMFASettings(); err == nil { + settings = loaded + } + } + return db.SessionAwaitingMFA(settings, authUser.DBUser, authUser.Session) +} + func determineMFALevel(resource rbac.Resource, action rbac.Action) rbac.MFALevel { permission := fmt.Sprintf("convox:%s:%s", resource.String(), action.String()) mfaLevel, ok := rbac.MFARequirements[permission] diff --git a/internal/gateway/routes/mfa_pending_test.go b/internal/gateway/routes/mfa_pending_test.go new file mode 100644 index 00000000..868f5972 --- /dev/null +++ b/internal/gateway/routes/mfa_pending_test.go @@ -0,0 +1,375 @@ +package routes_test + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/pquerna/otp/totp" + + "github.com/DocSpring/rack-gateway/internal/gateway/audit" + "github.com/DocSpring/rack-gateway/internal/gateway/auth" + "github.com/DocSpring/rack-gateway/internal/gateway/auth/mfa" + "github.com/DocSpring/rack-gateway/internal/gateway/config" + "github.com/DocSpring/rack-gateway/internal/gateway/db" + "github.com/DocSpring/rack-gateway/internal/gateway/deps" + "github.com/DocSpring/rack-gateway/internal/gateway/proxy" + "github.com/DocSpring/rack-gateway/internal/gateway/rbac" + "github.com/DocSpring/rack-gateway/internal/gateway/routes" + "github.com/DocSpring/rack-gateway/internal/gateway/settings" + "github.com/DocSpring/rack-gateway/internal/gateway/testutil/dbtest" + "github.com/DocSpring/rack-gateway/internal/gateway/token" +) + +const mfaTestHost = "gateway.example.com" + +type mfaRouteEnv struct { + router *gin.Engine + database *db.Database + sessions *auth.SessionManager + mfaService *mfa.Service + user *db.User + totpSecret string +} + +// newMFARouteEnv builds the full router against a fresh test database. When enrolledEmail is set, +// an admin user with a confirmed TOTP factor is created before the RBAC manager loads users. +func newMFARouteEnv(t *testing.T, enrolledEmail string) *mfaRouteEnv { + t.Helper() + gin.SetMode(gin.TestMode) + database := dbtest.NewDatabase(t) + + rack := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte("[]")) + })) + t.Cleanup(rack.Close) + + sessions := auth.NewSessionManager(database, "test-secret", &auth.StaticTTLProvider{TTL: time.Hour}) + mfaService, err := mfa.NewService(database, "RG", 30*24*time.Hour, 10*time.Minute, []byte("pepper"), + "", "", "", "", nil) + if err != nil { + t.Fatal(err) + } + env := &mfaRouteEnv{database: database, sessions: sessions, mfaService: mfaService} + if enrolledEmail != "" { + env.enrollUser(t, enrolledEmail) + } + + rbacManager, err := rbac.NewDBManager(database, "example.com") + if err != nil { + t.Fatal(err) + } + tokenService := token.NewService(database) + cfg := &config.Config{ + Domain: mfaTestHost, + SessionSecret: "test-secret", + Racks: map[string]config.RackConfig{ + "default": {Name: "default", URL: rack.URL, Username: "convox", APIKey: "rack-key", Enabled: true}, + }, + } + auditLogger := audit.NewLogger(database) + settingsService := settings.NewService(database) + gateway := &deps.Gateway{ + Config: cfg, + Database: database, + RBACManager: rbacManager, + SessionManager: sessions, + AuthService: auth.NewAuthService(tokenService, database, sessions), + TokenService: tokenService, + MFAService: mfaService, + MFASettings: &db.MFASettings{RequireAllUsers: true, StepUpWindowMinutes: 10, TrustedDeviceTTLDays: 30}, + SettingsService: settingsService, + AuditLogger: auditLogger, + ProxyHandler: proxy.NewHandler(cfg, rbacManager, auditLogger, database, settingsService, nil, + "default", "default", nil, mfaService, sessions), + } + router := gin.New() + if err := router.SetTrustedProxies(nil); err != nil { + t.Fatal(err) + } + routes.Setup(router, &routes.Config{Gateway: gateway}) + env.router = router + return env +} + +// enrollUser creates an admin user with a confirmed TOTP factor. +func (e *mfaRouteEnv) enrollUser(t *testing.T, email string) { + t.Helper() + user, err := e.database.CreateUser(email, "Test User", []string{"admin"}) + if err != nil { + t.Fatal(err) + } + start, err := e.mfaService.StartTOTPEnrollment(user) + if err != nil { + t.Fatal(err) + } + // Use the previous TOTP step so codes generated "now" during the test aren't replays. + code, err := totp.GenerateCode(start.Secret, time.Now().Add(-30*time.Second)) + if err != nil { + t.Fatal(err) + } + if err := e.mfaService.ConfirmTOTP(user, start.MethodID, code); err != nil { + t.Fatal(err) + } + e.user, err = e.database.GetUser(email) + if err != nil { + t.Fatal(err) + } + e.totpSecret = start.Secret +} + +type webSession struct { + token string + session *db.UserSession + headers map[string]string +} + +// newSession creates a web session as the OAuth callback does (MFA not yet verified). +func (e *mfaRouteEnv) newSession(t *testing.T, user *db.User) *webSession { + t.Helper() + sessionToken, session, err := e.sessions.CreateSession(user, auth.SessionMetadata{Channel: "web"}) + if err != nil { + t.Fatal(err) + } + csrf, err := e.sessions.DeriveCSRFToken(sessionToken) + if err != nil { + t.Fatal(err) + } + return &webSession{ + token: sessionToken, + session: session, + headers: map[string]string{"Cookie": "session_token=" + sessionToken, "X-CSRF-Token": csrf}, + } +} + +// markVerified marks the session's login MFA as complete. Without stepUp, the step-up +// timestamp is moved outside the step-up window. +func (e *mfaRouteEnv) markVerified(t *testing.T, s *webSession, stepUp bool) { + t.Helper() + now := time.Now() + if err := e.database.UpdateSessionMFAVerified(s.session.ID, now, nil); err != nil { + t.Fatal(err) + } + if !stepUp { + if err := e.database.UpdateSessionRecentStepUp(s.session.ID, now.Add(-time.Hour)); err != nil { + t.Fatal(err) + } + } +} + +func (e *mfaRouteEnv) currentCode(t *testing.T) string { + t.Helper() + code, err := totp.GenerateCode(e.totpSecret, time.Now()) + if err != nil { + t.Fatal(err) + } + return code +} + +func (e *mfaRouteEnv) do(method, path, body string, headers map[string]string) *httptest.ResponseRecorder { + var reader *strings.Reader + if body != "" { + reader = strings.NewReader(body) + } else { + reader = strings.NewReader("") + } + req := httptest.NewRequest(method, path, reader) + if body != "" { + req.Header.Set("Content-Type", "application/json") + } + req.Host = mfaTestHost + req.RemoteAddr = "10.1.2.3:5555" + req.Header.Set("User-Agent", "mfa-route-test") + for key, value := range headers { + req.Header.Set(key, value) + } + recorder := httptest.NewRecorder() + e.router.ServeHTTP(recorder, req) + return recorder +} + +func withHeader(headers map[string]string, key, value string) map[string]string { + out := make(map[string]string, len(headers)+1) + for k, v := range headers { + out[k] = v + } + out[key] = value + return out +} + +func bearer(s *webSession) map[string]string { + return map[string]string{"Authorization": "Bearer " + s.token} +} + +func assertStatus(t *testing.T, w *httptest.ResponseRecorder, want int, label string) { + t.Helper() + if w.Code != want { + t.Fatalf("%s: expected %d, got %d: %s", label, want, w.Code, w.Body.String()) + } +} + +func TestPendingMFASessionOnlyReachesChallengeRoutes(t *testing.T) { + e := newMFARouteEnv(t, "pending@example.com") + s := e.newSession(t, e.user) + + blocked := []struct{ method, path, body string }{ + {"GET", "/api/v1/users", ""}, + {"POST", "/api/v1/auth/mfa/enroll/totp/start", "{}"}, + {"POST", "/api/v1/api-tokens", `{"name":"x","permissions":["convox:app:list"]}`}, + {"POST", "/api/v1/auth/mfa/backup-codes/regenerate", "{}"}, + } + for _, tc := range blocked { + w := e.do(tc.method, tc.path, tc.body, s.headers) + assertStatus(t, w, http.StatusUnauthorized, tc.method+" "+tc.path) + if !strings.Contains(w.Body.String(), "mfa_step_up_required") { + t.Fatalf("%s %s: expected mfa_step_up_required, got %s", tc.method, tc.path, w.Body.String()) + } + } + + assertStatus(t, e.do("GET", "/api/v1/info", "", s.headers), http.StatusOK, "GET /api/v1/info") + assertStatus(t, e.do("GET", "/api/v1/auth/mfa/status", "", s.headers), http.StatusOK, "GET mfa status") + + // The same session token used as a Bearer credential on the CLI proxy is refused too. + w := e.do("GET", "/api/v1/rack-proxy/apps", "", bearer(s)) + assertStatus(t, w, http.StatusUnauthorized, "rack-proxy") + if !strings.Contains(w.Body.String(), "must be completed for this session") { + t.Fatalf("rack-proxy: expected pending-MFA denial, got %s", w.Body.String()) + } + + methods, err := e.database.ListMFAMethods(e.user.ID) + if err != nil { + t.Fatal(err) + } + if len(methods) != 1 { + t.Fatalf("pending session must not be able to add MFA methods, have %d", len(methods)) + } +} + +func TestPendingMFASessionUnlocksAfterVerification(t *testing.T) { + e := newMFARouteEnv(t, "verify@example.com") + s := e.newSession(t, e.user) + + body := fmt.Sprintf(`{"method":"totp","code":%q}`, e.currentCode(t)) + assertStatus(t, e.do("POST", "/api/v1/auth/mfa/verify", body, s.headers), http.StatusOK, "verify") + + assertStatus(t, e.do("GET", "/api/v1/users", "", s.headers), http.StatusOK, "GET users after verify") + assertStatus(t, e.do("GET", "/api/v1/rack-proxy/apps", "", bearer(s)), http.StatusOK, "rack-proxy after verify") +} + +func TestPendingMFASessionCanCompleteChallengeInline(t *testing.T) { + e := newMFARouteEnv(t, "inline@example.com") + s := e.newSession(t, e.user) + + headers := withHeader(s.headers, "X-MFA-TOTP", e.currentCode(t)) + assertStatus(t, e.do("GET", "/api/v1/users", "", headers), http.StatusOK, "GET users with inline TOTP") + + result, err := e.sessions.ValidateSession(s.token, "10.1.2.3", "mfa-route-test") + if err != nil { + t.Fatal(err) + } + if result.Session.MFAVerifiedAt == nil { + t.Fatal("expected inline MFA to mark the session as verified") + } + assertStatus(t, e.do("GET", "/api/v1/users", "", s.headers), http.StatusOK, "GET users afterwards") +} + +func TestAddingFactorRequiresRecentStepUp(t *testing.T) { + e := newMFARouteEnv(t, "stepup@example.com") + s := e.newSession(t, e.user) + e.markVerified(t, s, false) + + w := e.do("POST", "/api/v1/auth/mfa/enroll/totp/start", "{}", s.headers) + assertStatus(t, w, http.StatusUnauthorized, "enroll without step-up") + if !strings.Contains(w.Body.String(), "mfa_step_up_required") { + t.Fatalf("expected mfa_step_up_required, got %s", w.Body.String()) + } + + if err := e.database.UpdateSessionRecentStepUp(s.session.ID, time.Now()); err != nil { + t.Fatal(err) + } + w = e.do("POST", "/api/v1/auth/mfa/enroll/totp/start", "{}", s.headers) + assertStatus(t, w, http.StatusOK, "enroll with step-up") + + var resp struct { + BackupCodes []string `json:"backup_codes"` + } + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatal(err) + } + if len(resp.BackupCodes) != 0 { + t.Fatalf("adding a second factor must not reissue backup codes, got %d", len(resp.BackupCodes)) + } +} + +func TestFirstEnrollmentNeedsNoStepUp(t *testing.T) { + e := newMFARouteEnv(t, "") + user, err := e.database.CreateUser("new@example.com", "New User", []string{"admin"}) + if err != nil { + t.Fatal(err) + } + s := e.newSession(t, user) + + w := e.do("POST", "/api/v1/auth/mfa/enroll/totp/start", "{}", s.headers) + assertStatus(t, w, http.StatusOK, "first enrollment start") + var start struct { + MethodID int64 `json:"method_id"` + Secret string `json:"secret"` + BackupCodes []string `json:"backup_codes"` + } + if err := json.Unmarshal(w.Body.Bytes(), &start); err != nil { + t.Fatal(err) + } + if len(start.BackupCodes) == 0 { + t.Fatal("expected backup codes on first enrollment") + } + + code, err := totp.GenerateCode(start.Secret, time.Now()) + if err != nil { + t.Fatal(err) + } + body := fmt.Sprintf(`{"method_id":%d,"code":%q}`, start.MethodID, code) + assertStatus(t, e.do("POST", "/api/v1/auth/mfa/enroll/totp/confirm", body, s.headers), http.StatusOK, + "first enrollment confirm") + + enrolled, err := e.database.GetUser(user.Email) + if err != nil { + t.Fatal(err) + } + if !enrolled.MFAEnrolled { + t.Fatal("expected user to be enrolled") + } +} + +func TestMFAChangesAreAudited(t *testing.T) { + e := newMFARouteEnv(t, "audited@example.com") + s := e.newSession(t, e.user) + e.markVerified(t, s, true) + + assertStatus(t, e.do("POST", "/api/v1/auth/mfa/backup-codes/regenerate", "{}", s.headers), http.StatusOK, + "regenerate backup codes") + + methods, err := e.database.ListMFAMethods(e.user.ID) + if err != nil || len(methods) == 0 { + t.Fatalf("expected an MFA method, err=%v", err) + } + deletePath := fmt.Sprintf("/api/v1/auth/mfa/methods/%d", methods[0].ID) + headers := withHeader(s.headers, "X-MFA-TOTP", e.currentCode(t)) + assertStatus(t, e.do("DELETE", deletePath, "", headers), http.StatusOK, "delete MFA method") + + for _, action := range []string{"mfa_backup_codes.generate", "mfa_method.delete"} { + var count int + row := e.database.DB().QueryRow(`SELECT COUNT(*) FROM audit.audit_event WHERE action = $1`, action) + if err := row.Scan(&count); err != nil { + t.Fatal(err) + } + if count != 1 { + t.Fatalf("expected one %s audit event, got %d", action, count) + } + } +} diff --git a/internal/gateway/routes/routes.go b/internal/gateway/routes/routes.go index e65ffd8a..27560166 100644 --- a/internal/gateway/routes/routes.go +++ b/internal/gateway/routes/routes.go @@ -43,6 +43,7 @@ func Setup(router *gin.Engine, cfg *Config) { authenticated := api.Group("") authenticated.Use(middleware.Authenticated(cfg.AuthService, cfg.RBACManager)) authenticated.Use(middleware.RequireMFAEnrollmentWeb(cfg.Database, cfg.MFASettings)) + authenticated.Use(middleware.RequireVerifiedMFASession(cfg.MFAService, cfg.Database, cfg.MFASettings)) authenticated.Use(middleware.EnforceMFARequirements(cfg.MFAService, cfg.Database, cfg.MFASettings)) registerMFARoutes(authenticated, cfg, h) From e4e5bc8ad1f60d0b64f663bf4d4c667c6451ef35 Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Fri, 9 Oct 2026 15:17:18 +1300 Subject: [PATCH 03/28] Reject env var names and values that can break the KEY=VALUE format PUT /api/v1/apps/:app/env only validated the submitted key, then joined values with newlines for the rack API. A value like "x\nDATABASE_URL=postgres://evil" injected a second line that Convox applied (last duplicate wins). That bypassed protected env vars and the secrets:set permission, and the audit log only recorded the harmless key. MergeEnv now requires keys to match [A-Za-z_][A-Za-z0-9_]* and rejects CR, LF and NUL in values. The handler returns 400 with an explanation. --- internal/gateway/envutil/envutil.go | 21 ++++++++++ internal/gateway/envutil/envutil_test.go | 42 +++++++++++++++++++- internal/gateway/handlers/api_handler_env.go | 4 ++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/internal/gateway/envutil/envutil.go b/internal/gateway/envutil/envutil.go index da55359a..8648ae3a 100644 --- a/internal/gateway/envutil/envutil.go +++ b/internal/gateway/envutil/envutil.go @@ -9,6 +9,7 @@ import ( "fmt" "net/http" "net/url" + "regexp" "sort" "strings" "time" @@ -28,8 +29,22 @@ var ( // ErrMaskedSecretWithoutBase is returned when a masked secret value is provided without an existing // value to preserve. ErrMaskedSecretWithoutBase = errors.New("masked secret provided without existing value") + // ErrInvalidEnvEntry is returned for keys that aren't valid env var names, or values containing line + // breaks or NUL. The rack API takes env as newline-separated KEY=VALUE lines, so a line break in a + // value would inject extra variables. + ErrInvalidEnvEntry = errors.New("invalid environment variable name or value") ) +var envKeyPattern = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) + +// ValidateEnvEntry checks that key is a valid env var name and value cannot break the KEY=VALUE line format. +func ValidateEnvEntry(key, value string) error { + if !envKeyPattern.MatchString(key) || strings.ContainsAny(value, "\r\n\x00") { + return ErrInvalidEnvEntry + } + return nil +} + // EnvDiff describes a change applied to an environment variable. type EnvDiff struct { Key string @@ -98,6 +113,9 @@ func processRemovals( if key == "" { continue } + if err := ValidateEnvEntry(key, ""); err != nil { + return nil, err + } if opts.IsProtectedKey != nil && opts.IsProtectedKey(key) { return nil, ErrProtectedEnvModification } @@ -152,6 +170,9 @@ func processSetOperation( removedOld map[string]string, opts MergeOptions, ) (*EnvDiff, error) { + if err := ValidateEnvEntry(key, value); err != nil { + return nil, err + } if err := validateProtectedKey(key, value, merged, removedOld, opts); err != nil { return nil, err } diff --git a/internal/gateway/envutil/envutil_test.go b/internal/gateway/envutil/envutil_test.go index f3cb1cb0..3d588637 100644 --- a/internal/gateway/envutil/envutil_test.go +++ b/internal/gateway/envutil/envutil_test.go @@ -1,6 +1,9 @@ package envutil -import "testing" +import ( + "errors" + "testing" +) func TestMergeEnvMaskedSecretRequiresExistingValue(t *testing.T) { base := map[string]string{} @@ -67,3 +70,40 @@ func TestMergeEnvMaskedSecretByViewerNoSecretPermission(t *testing.T) { t.Fatalf("expected single diff for FOO, got %#v", diffs) } } + +func TestMergeEnvRejectsLineBreakInjection(t *testing.T) { + base := map[string]string{"DATABASE_URL": "postgres://real/db"} + protected := func(key string) bool { return key == "DATABASE_URL" } + + for name, set := range map[string]map[string]string{ + "newline in value": {"ZZZ_NOTE": "x\nDATABASE_URL=postgres://evil/x"}, + "carriage return in value": {"ZZZ_NOTE": "x\rDATABASE_URL=postgres://evil/x"}, + "NUL in value": {"ZZZ_NOTE": "x\x00y"}, + "equals in key": {"A=B": "x"}, + "newline in key": {"A\nDATABASE_URL": "x"}, + "leading digit key": {"1ABC": "x"}, + } { + _, _, err := MergeEnv(base, set, nil, MergeOptions{IsProtectedKey: protected}) + if !errors.Is(err, ErrInvalidEnvEntry) { + t.Fatalf("%s: expected ErrInvalidEnvEntry, got %v", name, err) + } + } + + _, _, err := MergeEnv(base, nil, []string{"BAD KEY"}, MergeOptions{}) + if !errors.Is(err, ErrInvalidEnvEntry) { + t.Fatalf("remove with invalid key: expected ErrInvalidEnvEntry, got %v", err) + } +} + +func TestMergeEnvAllowsOrdinaryValues(t *testing.T) { + merged, _, err := MergeEnv(map[string]string{}, map[string]string{ + "_PRIVATE": "ok", + "FEATURE_X": "a=b; c, d=\"e\" \t tab", + }, nil, MergeOptions{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if merged["FEATURE_X"] != "a=b; c, d=\"e\" \t tab" { + t.Fatalf("value changed: %q", merged["FEATURE_X"]) + } +} diff --git a/internal/gateway/handlers/api_handler_env.go b/internal/gateway/handlers/api_handler_env.go index 191b5b97..d26173e5 100644 --- a/internal/gateway/handlers/api_handler_env.go +++ b/internal/gateway/handlers/api_handler_env.go @@ -378,6 +378,10 @@ func (_ *APIHandler) respondMergeError(c *gin.Context, mergeErr error) { c.JSON(http.StatusForbidden, gin.H{"error": "You don't have permission to modify secrets."}) case errors.Is(mergeErr, envutil.ErrProtectedEnvModification): c.JSON(http.StatusForbidden, gin.H{"error": "This environment variable is protected and cannot be changed."}) + case errors.Is(mergeErr, envutil.ErrInvalidEnvEntry): + c.JSON(http.StatusBadRequest, gin.H{ + "error": "Env var names must match [A-Za-z_][A-Za-z0-9_]* and values cannot contain line breaks.", + }) case errors.Is(mergeErr, envutil.ErrMaskedSecretWithoutBase): c.JSON(http.StatusBadRequest, gin.H{"error": "Masked secret value submitted without an existing secret."}) default: From d648908b389aa701c3fbfa402dd6c34a72f2496b Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Fri, 9 Oct 2026 15:30:34 +1300 Subject: [PATCH 04/28] Proxy: forward an allowlist of client headers and set the rack actor The proxy copied every client header to the rack except a five-entry denylist. The rack trusts several of those headers because the gateway authenticates as admin with the rack password: - Image/Volumes/Privileged on a process run let ops or an approved CI token start an arbitrary image with host paths mounted - X-Convox-Actor forged the rack's own audit trail - X-Convox-TID switched tenant namespace resolution - Client headers now pass through an explicit allowlist derived from the Convox SDK (sdk.Client.Headers, stdsdk.Client.Request) and the header tags on structs.LogsOptions, ProcessExecOptions, ProcessRunOptions and ObjectOptions. Credentials, cookies, actor and tenant overrides, X-Forwarded-*, hop-by-hop headers, Accept-Encoding, env headers and gateway-internal headers are dropped, on both HTTP and WebSocket requests. - Run options that escape the release image or scheduling constraints (Image, Volumes, Privileged, Node-Labels, Node-Affinity, Run-Tolerations, System-Critical, Run-Annotations, Run-Labels) need the new convox:process:run_privileged permission, which only the admin wildcard covers. API tokens can never use them. Requests without that permission get 403 and never reach the rack. - The gateway sets X-Convox-Actor to the user's email, or token: for API tokens. - New StripInternalHeaders middleware runs first and removes client-supplied X-Audit-Resource, X-Release-Created, X-Original-Path, X-User-*, X-API-Token-*, X-Auth-Source, X-RBAC-Decision and X-Rack-* headers, which the audit logger and request logger read. - buildTargetURL escapes the authorized path, so %3F/%23 can't truncate the route the rack sees. Paths with control characters, '?', '#' or dot segments are rejected before route matching. - The WebSocket dial follows redirects only on the rack's own host, so the rack credential is never sent anywhere else. --- .../gateway/middleware/internal_headers.go | 31 +++ .../middleware/internal_headers_test.go | 34 +++ internal/gateway/proxy/forward.go | 11 +- internal/gateway/proxy/forward_headers.go | 151 +++++++++++ .../gateway/proxy/forward_headers_test.go | 240 ++++++++++++++++++ internal/gateway/proxy/forward_helpers.go | 31 +-- internal/gateway/proxy/handler.go | 3 + internal/gateway/proxy/websocket.go | 54 +--- internal/gateway/rbac/action_string.go | 5 +- internal/gateway/rbac/constants.go | 7 +- internal/gateway/routes/setup_helpers.go | 2 + 11 files changed, 492 insertions(+), 77 deletions(-) create mode 100644 internal/gateway/middleware/internal_headers.go create mode 100644 internal/gateway/middleware/internal_headers_test.go create mode 100644 internal/gateway/proxy/forward_headers.go create mode 100644 internal/gateway/proxy/forward_headers_test.go diff --git a/internal/gateway/middleware/internal_headers.go b/internal/gateway/middleware/internal_headers.go new file mode 100644 index 00000000..6e7e64b7 --- /dev/null +++ b/internal/gateway/middleware/internal_headers.go @@ -0,0 +1,31 @@ +package middleware + +import "github.com/gin-gonic/gin" + +// gatewayInternalHeaders are request headers the gateway sets itself to pass identity and audit details +// between middleware, handlers and the audit logger. Clients must never be able to supply them: they +// would spoof the user, token, resource, RBAC decision or path recorded in the audit log. +var gatewayInternalHeaders = []string{ + "X-Audit-Resource", + "X-Release-Created", + "X-Original-Path", + "X-User-Email", + "X-User-Name", + "X-API-Token-ID", + "X-API-Token-Name", + "X-Auth-Source", + "X-RBAC-Decision", + "X-Rack-Alias", + "X-Rack-Name", +} + +// StripInternalHeaders removes client-supplied copies of gateway-internal headers. +// It must run before any other middleware. +func StripInternalHeaders() gin.HandlerFunc { + return func(c *gin.Context) { + for _, name := range gatewayInternalHeaders { + c.Request.Header.Del(name) + } + c.Next() + } +} diff --git a/internal/gateway/middleware/internal_headers_test.go b/internal/gateway/middleware/internal_headers_test.go new file mode 100644 index 00000000..cf9d85ef --- /dev/null +++ b/internal/gateway/middleware/internal_headers_test.go @@ -0,0 +1,34 @@ +package middleware + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" +) + +func TestStripInternalHeadersRemovesClientCopies(t *testing.T) { + gin.SetMode(gin.TestMode) + router := gin.New() + router.Use(StripInternalHeaders()) + + var seen http.Header + router.GET("/x", func(c *gin.Context) { + seen = c.Request.Header.Clone() + c.Status(http.StatusNoContent) + }) + + req := httptest.NewRequest(http.MethodGet, "/x", nil) + for _, name := range gatewayInternalHeaders { + req.Header.Set(name, "spoofed") + } + req.Header.Set("X-Request-ID", "kept") + router.ServeHTTP(httptest.NewRecorder(), req) + + for _, name := range gatewayInternalHeaders { + require.Emptyf(t, seen.Get(name), "%s must be stripped", name) + } + require.Equal(t, "kept", seen.Get("X-Request-ID")) +} diff --git a/internal/gateway/proxy/forward.go b/internal/gateway/proxy/forward.go index 4ee9eb19..1058b5ab 100644 --- a/internal/gateway/proxy/forward.go +++ b/internal/gateway/proxy/forward.go @@ -269,7 +269,16 @@ func (h *Handler) forwardRequest( path string, authUser *auth.User, ) (int, error) { - targetURL := buildTargetURL(rack, path, r.URL.RawQuery) + targetURL, err := buildTargetURL(rack, path, r.URL.RawQuery) + if err != nil { + return 0, err + } + + if disallowed := h.disallowedRunOptions(r, authUser); len(disallowed) > 0 { + msg := fmt.Sprintf("only admins can use these process options: %s", strings.Join(disallowed, ", ")) + http.Error(w, msg, http.StatusForbidden) + return http.StatusForbidden, nil + } if strings.Contains(strings.ToLower(r.Header.Get("Connection")), "upgrade") && strings.ToLower(r.Header.Get("Upgrade")) == "websocket" { diff --git a/internal/gateway/proxy/forward_headers.go b/internal/gateway/proxy/forward_headers.go new file mode 100644 index 00000000..f3636136 --- /dev/null +++ b/internal/gateway/proxy/forward_headers.go @@ -0,0 +1,151 @@ +package proxy + +import ( + "encoding/base64" + "fmt" + "net/http" + "net/url" + "sort" + "strconv" + "strings" + + "github.com/google/uuid" + + "github.com/DocSpring/rack-gateway/internal/gateway/auth" + "github.com/DocSpring/rack-gateway/internal/gateway/config" + "github.com/DocSpring/rack-gateway/internal/gateway/rbac" +) + +type headerSet map[string]struct{} + +func newHeaderSet(names ...string) headerSet { + set := make(headerSet, len(names)) + for _, name := range names { + set[http.CanonicalHeaderKey(name)] = struct{}{} + } + return set +} + +func (s headerSet) has(name string) bool { + _, ok := s[http.CanonicalHeaderKey(name)] + return ok +} + +// forwardedClientHeaders lists the client request headers the gateway passes on to the rack API. +// Derived from the Convox SDK (sdk.Client.Headers, stdsdk.Client.Request) and the `header:"..."` option +// tags in convox pkg/structs (LogsOptions, ProcessExecOptions, ProcessRunOptions, ObjectOptions). +// +// Everything else is dropped: client credentials and cookies, actor and tenant overrides (X-Convox-Actor, +// X-Convox-TID), X-Forwarded-*, hop-by-hop headers, Accept-Encoding (so the transport decompresses responses +// before the gateway masks secrets in them), gateway-internal headers, and env headers (env changes must go +// through the release body, where the gateway checks protected and secret keys). +var forwardedClientHeaders = newHeaderSet( + // Sent by the SDK on every request + "Accept", "Content-Type", "User-Agent", "Version", + // Logs + "Filter", "Follow", "Maxlogrequests", "Prefix", "Previous", "Since", "Tail", + // Exec (the SDK sends the exec command as a header) + "Command", "Disable-Stdin", "Entrypoint", "Height", "Tty", "Width", + // Run + "Cpu", "Cpu-Limit", "Gpu", "Gpu-Vendor", "Memory", "Memory-Limit", "Release", "Retain", + "Termination-Grace", "Use-Service-Lifecycle", "Use-Service-Volume", + // Object uploads + "Public", +) + +// privilegedRunHeaders are process run options that let a one-off process escape the app's release image or +// scheduling constraints: a custom image, host-path volumes, privileged mode, node placement and tolerations, +// system-critical priority, and arbitrary pod labels/annotations (which can route service traffic to the pod +// or relax security profiles). Only callers holding convox:process:run_privileged may send them, and API +// tokens never may. +var privilegedRunHeaders = newHeaderSet( + "Image", "Volumes", "Privileged", "Node-Labels", "Node-Affinity", "Run-Tolerations", + "System-Critical", "Run-Annotations", "Run-Labels", +) + +// copyForwardedHeaders copies only allowlisted client headers into dst. +func copyForwardedHeaders(dst, src http.Header) { + for key, values := range src { + canonical := http.CanonicalHeaderKey(key) + if !forwardedClientHeaders.has(canonical) && !privilegedRunHeaders.has(canonical) { + continue + } + for _, value := range values { + dst.Add(canonical, value) + } + } +} + +// setGatewayHeaders adds the rack credential and the identity the rack should record for the request. +// The actor header replaces anything the client sent, so the rack's own audit trail names the real caller. +func setGatewayHeaders(dst http.Header, rack config.RackConfig, authUser *auth.User) { + credentials := base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", rack.Username, rack.APIKey))) + dst.Set("Authorization", "Basic "+credentials) + dst.Set("X-Request-ID", uuid.New().String()) + if authUser == nil { + return + } + dst.Set("X-User-Email", authUser.Email) + dst.Set("X-Convox-Actor", rackActor(authUser)) +} + +// rackActor names the caller in the rack's audit trail: the user's email, or token: for API tokens. +func rackActor(authUser *auth.User) string { + if !authUser.IsAPIToken { + return authUser.Email + } + name := strings.TrimSpace(authUser.TokenName) + if name == "" && authUser.TokenID != nil { + name = strconv.FormatInt(*authUser.TokenID, 10) + } + return "token:" + name +} + +// disallowedRunOptions returns the privileged run options in r that the caller may not use, sorted. +func (h *Handler) disallowedRunOptions(r *http.Request, authUser *auth.User) []string { + var requested []string + for key := range r.Header { + if privilegedRunHeaders.has(key) { + requested = append(requested, http.CanonicalHeaderKey(key)) + } + } + if len(requested) == 0 { + return nil + } + if authUser != nil && !authUser.IsAPIToken && h.callerCan(r, rbac.ResourceProcess, rbac.ActionRunPrivileged) { + return nil + } + sort.Strings(requested) + return requested +} + +// buildTargetURL constructs the rack URL for a proxied request. The path is the decoded path the gateway +// authorized; it is re-escaped so characters like '?' and '#' cannot change the route the rack sees. +func buildTargetURL(rack config.RackConfig, path string, rawQuery string) (string, error) { + base, err := url.Parse(strings.TrimRight(rack.URL, "/")) + if err != nil { + return "", fmt.Errorf("invalid rack URL: %w", err) + } + target := *base + target.Path = base.Path + "/" + strings.TrimLeft(path, "/") + target.RawPath = "" + target.RawQuery = rawQuery + target.Fragment = "" + return target.String(), nil +} + +// isSafeRackPath rejects decoded paths the rack could interpret differently from the gateway's route +// matcher: control characters, '?' or '#' smuggled in via percent-encoding, and dot segments. +func isSafeRackPath(path string) bool { + for _, r := range path { + if r < 0x20 || r == 0x7f || r == '?' || r == '#' { + return false + } + } + for _, segment := range strings.Split(path, "/") { + if segment == "." || segment == ".." { + return false + } + } + return true +} diff --git a/internal/gateway/proxy/forward_headers_test.go b/internal/gateway/proxy/forward_headers_test.go new file mode 100644 index 00000000..665fe12f --- /dev/null +++ b/internal/gateway/proxy/forward_headers_test.go @@ -0,0 +1,240 @@ +package proxy + +import ( + "context" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync" + "testing" + "time" + + "github.com/gorilla/websocket" + "github.com/stretchr/testify/require" + + "github.com/DocSpring/rack-gateway/internal/gateway/auth" + "github.com/DocSpring/rack-gateway/internal/gateway/config" + "github.com/DocSpring/rack-gateway/internal/gateway/db" + "github.com/DocSpring/rack-gateway/internal/gateway/rbac" +) + +// recordingRack captures the requests the gateway forwards to the rack. +type recordingRack struct { + mu sync.Mutex + requests []*http.Request +} + +func (rr *recordingRack) handler(w http.ResponseWriter, r *http.Request) { + rr.mu.Lock() + rr.requests = append(rr.requests, r.Clone(context.Background())) + rr.mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"id":"P1"}`)) +} + +func (rr *recordingRack) last(t *testing.T) *http.Request { + t.Helper() + rr.mu.Lock() + defer rr.mu.Unlock() + require.NotEmpty(t, rr.requests, "expected the request to reach the rack") + return rr.requests[len(rr.requests)-1] +} + +func (rr *recordingRack) count() int { + rr.mu.Lock() + defer rr.mu.Unlock() + return len(rr.requests) +} + +func newRecordingProxy(t *testing.T) (*Handler, *db.Database, *recordingRack) { + t.Helper() + rack := &recordingRack{} + h, database, mgr, cleanup := newProxyWithRackServer(t, rack.handler) + t.Cleanup(cleanup) + for userEmail, role := range map[string]string{ + "admin@test.com": "admin", + "ops@test.com": "ops", + "deployer@test.com": "deployer", + } { + require.NoError(t, mgr.SaveUser(userEmail, &rbac.UserConfig{Name: role, Roles: []string{role}})) + } + return h, database, rack +} + +func withPath(req *http.Request, method, path string) *http.Request { + clone := req.Clone(req.Context()) + clone.Method = method + clone.URL = &url.URL{Path: path} + clone.RequestURI = "" + return clone +} + +func TestProxyForwardsOnlyAllowlistedHeaders(t *testing.T) { + h, database, rack := newRecordingProxy(t) + req := withPath(requestAs(t, database, "deployer@test.com"), http.MethodGet, "/apps/myapp/processes") + spoofed := map[string]string{ + "X-Convox-Actor": "ceo@example.com", + "Convox-Actor": "ceo@example.com", + "X-Convox-TID": "other-tenant", + "Convox-TID": "other-tenant", + "X-Forwarded-For": "10.0.0.1", + "Cookie": "session_token=abc", + "Proxy-Authorization": "Basic Zm9vOmJhcg==", + "X-Something-Else": "1", + "Env": "SECRET=1", + } + for k, v := range spoofed { + req.Header.Set(k, v) + } + req.Header.Set("Since", "1h") + + rr := httptest.NewRecorder() + h.ProxyToRack(rr, req) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + + forwarded := rack.last(t).Header + require.Equal(t, "deployer@test.com", forwarded.Get("X-Convox-Actor"), "gateway must set the actor itself") + for name := range spoofed { + if name == "X-Convox-Actor" { + continue + } + require.Emptyf(t, forwarded.Get(name), "%s must not be forwarded", name) + } + require.Equal(t, "1h", forwarded.Get("Since")) + require.Equal(t, "Basic Y29udm94OnRva2Vu", forwarded.Get("Authorization")) +} + +func TestCopyForwardedHeadersDropsHopByHopHeaders(t *testing.T) { + src := http.Header{} + for _, name := range []string{ + "Connection", "Keep-Alive", "Te", "Trailer", "Transfer-Encoding", "Upgrade", + "Proxy-Connection", "Accept-Encoding", "X-Forwarded-Proto", "Authorization", + } { + src.Set(name, "x") + } + src.Set("Command", "rails db:migrate") + dst := http.Header{} + copyForwardedHeaders(dst, src) + require.Equal(t, http.Header{"Command": {"rails db:migrate"}}, dst) +} + +func TestPrivilegedRunOptionsRequireAdmin(t *testing.T) { + h, database, rack := newRecordingProxy(t) + const runPath = "/apps/myapp/services/web/processes" + + admin, err := database.GetUser("admin@test.com") + require.NoError(t, err) + tokenID := int64(42) + tokenUser := &auth.User{ + Email: admin.Email, IsAPIToken: true, TokenID: &tokenID, TokenName: "ci", + Permissions: []string{"convox:*:*"}, DBUser: admin, + } + tokenReq := httptest.NewRequest(http.MethodPost, runPath, nil) + tokenReq = tokenReq.WithContext(context.WithValue(tokenReq.Context(), auth.UserContextKey, tokenUser)) + + callers := map[string]*http.Request{ + "ops": requestAs(t, database, "ops@test.com"), + "deployer": requestAs(t, database, "deployer@test.com"), + "admin-owned wildcard": tokenReq, + } + for _, option := range []string{"Image", "Volumes", "Privileged", "Node-Labels", "Run-Tolerations"} { + for name, base := range callers { + req := withPath(base, http.MethodPost, runPath) + req.Header.Set("Command", "sleep 3600") + req.Header.Set(option, "x") + before := rack.count() + rr := httptest.NewRecorder() + h.ProxyToRack(rr, req) + require.Equalf(t, http.StatusForbidden, rr.Code, "%s with %s: %s", name, option, rr.Body.String()) + require.Equalf(t, before, rack.count(), "%s with %s must not reach the rack", name, option) + } + } + + req := withPath(requestAs(t, database, "admin@test.com"), http.MethodPost, runPath) + req.Header.Set("Command", "sleep 3600") + req.Header.Set("Image", "custom:latest") + rr := httptest.NewRecorder() + h.ProxyToRack(rr, req) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Equal(t, "custom:latest", rack.last(t).Header.Get("Image")) + + req = withPath(requestAs(t, database, "ops@test.com"), http.MethodPost, runPath) + req.Header.Set("Command", "sleep 3600") + req.Header.Set("Memory", "512") + rr = httptest.NewRecorder() + h.ProxyToRack(rr, req) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Equal(t, "512", rack.last(t).Header.Get("Memory")) +} + +func TestRackActorNamesTokens(t *testing.T) { + id := int64(7) + require.Equal(t, "me@example.com", rackActor(&auth.User{Email: "me@example.com"})) + token := &auth.User{Email: "me@example.com", IsAPIToken: true, TokenName: "CircleCI"} + require.Equal(t, "token:CircleCI", rackActor(token)) + require.Equal(t, "token:7", rackActor(&auth.User{IsAPIToken: true, TokenID: &id})) +} + +func TestBuildTargetURLEscapesPath(t *testing.T) { + rack := config.RackConfig{URL: "https://rack.internal:5443"} + got, err := buildTargetURL(rack, "/apps/myapp?x/releases/R1/promote", "a=b") + require.NoError(t, err) + require.Equal(t, "https://rack.internal:5443/apps/myapp%3Fx/releases/R1/promote?a=b", got) + + got, err = buildTargetURL(rack, "/apps/my#app/processes", "") + require.NoError(t, err) + require.Equal(t, "https://rack.internal:5443/apps/my%23app/processes", got) +} + +func TestIsSafeRackPath(t *testing.T) { + require.True(t, isSafeRackPath("/apps/myapp/processes")) + for _, path := range []string{ + "/apps/myapp?x/releases", + "/apps/my#app/processes", + "/apps/my\napp/processes", + "/apps/my\x7fapp/processes", + "/apps/../system", + "/apps/./processes", + } { + require.Falsef(t, isSafeRackPath(path), "%q", path) + } +} + +func TestProxyRejectsSmuggledPathCharacters(t *testing.T) { + h, database, rack := newRecordingProxy(t) + req := withPath(requestAs(t, database, "admin@test.com"), http.MethodPost, "/apps/myapp?x/releases/R1/promote") + rr := httptest.NewRecorder() + h.ProxyToRack(rr, req) + require.Equal(t, http.StatusNotFound, rr.Code) + require.Zero(t, rack.count()) +} + +func TestWebSocketDialRefusesCrossHostRedirect(t *testing.T) { + var elsewhereHit bool + elsewhere := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { + elsewhereHit = true + })) + defer elsewhere.Close() + + rackServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Location", strings.Replace(elsewhere.URL, "http://", "ws://", 1)+"/steal") + w.WriteHeader(http.StatusFound) + })) + defer rackServer.Close() + + wsURL, err := url.Parse(strings.Replace(rackServer.URL, "http://", "ws://", 1) + "/apps/a/processes/p/exec") + require.NoError(t, err) + dialer := &websocket.Dialer{HandshakeTimeout: 2 * time.Second} + h := &Handler{} + conn, resp, err := h.dialWithRedirects(dialer, wsURL, http.Header{"Authorization": {"Basic secret"}}) + if conn != nil { + _ = conn.Close() + } + if resp != nil && resp.Body != nil { + _ = resp.Body.Close() + } + require.Error(t, err) + require.Contains(t, err.Error(), "refusing websocket redirect") + require.False(t, elsewhereHit, "rack credential must not be sent to another host") +} diff --git a/internal/gateway/proxy/forward_helpers.go b/internal/gateway/proxy/forward_helpers.go index f37ec98d..a3766284 100644 --- a/internal/gateway/proxy/forward_helpers.go +++ b/internal/gateway/proxy/forward_helpers.go @@ -2,17 +2,13 @@ package proxy import ( "bytes" - "encoding/base64" "fmt" "io" "net/http" "strings" - "github.com/google/uuid" - "github.com/DocSpring/rack-gateway/internal/gateway/auth" "github.com/DocSpring/rack-gateway/internal/gateway/config" - "github.com/DocSpring/rack-gateway/internal/gateway/httputil" ) // prepareProxyRequest creates a new HTTP request for proxying to the Convox rack. @@ -33,20 +29,8 @@ func prepareProxyRequest( return nil, fmt.Errorf("failed to create proxy request: %w", err) } - httputil.CopyHeaders( - proxyReq.Header, - r.Header, - "authorization", - "env", - "environment", - "release-env", - "x-audit-resource", - ) - - proxyReq.Header.Set("Authorization", fmt.Sprintf("Basic %s", - base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", rack.Username, rack.APIKey))))) - proxyReq.Header.Set("X-User-Email", authUser.Email) - proxyReq.Header.Set("X-Request-ID", uuid.New().String()) + copyForwardedHeaders(proxyReq.Header, r.Header) + setGatewayHeaders(proxyReq.Header, rack, authUser) return proxyReq, nil } @@ -68,14 +52,3 @@ func readRequestBody(r *http.Request) ([]byte, error) { return bodyBytes, nil } - -// buildTargetURL constructs the full target URL for the proxied request. -func buildTargetURL(rack config.RackConfig, path string, rawQuery string) string { - base := strings.TrimRight(rack.URL, "/") - p := "/" + strings.TrimLeft(path, "/") - targetURL := base + p - if rawQuery != "" { - targetURL += "?" + rawQuery - } - return targetURL -} diff --git a/internal/gateway/proxy/handler.go b/internal/gateway/proxy/handler.go index e7f781ef..d28d1e8b 100644 --- a/internal/gateway/proxy/handler.go +++ b/internal/gateway/proxy/handler.go @@ -310,6 +310,9 @@ func (h *Handler) handleForwardError( } func (h *Handler) isAllowedConvoxRoute(r *http.Request, rackPath string) bool { + if !isSafeRackPath(rackPath) { + return false + } methodForAllow := h.determineMethod(r) _, _, ok := rbac.MatchRackRoute(methodForAllow, rackPath) return ok diff --git a/internal/gateway/proxy/websocket.go b/internal/gateway/proxy/websocket.go index 23c470ca..e1839d46 100644 --- a/internal/gateway/proxy/websocket.go +++ b/internal/gateway/proxy/websocket.go @@ -2,7 +2,6 @@ package proxy import ( "context" - "encoding/base64" "fmt" "io" "log" @@ -12,7 +11,6 @@ import ( "strings" "time" - "github.com/google/uuid" "github.com/gorilla/websocket" "github.com/DocSpring/rack-gateway/internal/gateway/auth" @@ -44,11 +42,7 @@ func (h *Handler) proxyWebSocket( return 0, err } - userEmail := "" - if authUser != nil { - userEmail = authUser.Email - } - header := h.buildWebSocketHeaders(r, rack, userEmail, wsURL) + header := buildWebSocketHeaders(r, rack, authUser, wsURL) upstreamConn, resp, err := h.dialUpstreamWebSocket(r.Context(), wsURL, header, rack.URL) if err != nil { @@ -179,18 +173,16 @@ func (_ *Handler) prepareWebSocketURL(target string) (*url.URL, error) { return u, nil } -// buildWebSocketHeaders constructs headers for upstream WebSocket connection -func (h *Handler) buildWebSocketHeaders( +// buildWebSocketHeaders constructs headers for the upstream WebSocket connection. +func buildWebSocketHeaders( r *http.Request, rack config.RackConfig, - userEmail string, + authUser *auth.User, wsURL *url.URL, ) http.Header { header := http.Header{} - authValue := fmt.Sprintf("%s:%s", rack.Username, rack.APIKey) - header.Set("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(authValue))) - header.Set("X-User-Email", userEmail) - header.Set("X-Request-ID", uuid.New().String()) + copyForwardedHeaders(header, r.Header) + setGatewayHeaders(header, rack, authUser) scheme := "http" if strings.HasPrefix(rack.URL, "https") { @@ -198,41 +190,12 @@ func (h *Handler) buildWebSocketHeaders( } header.Set("Origin", fmt.Sprintf("%s://%s", scheme, wsURL.Host)) - h.copyClientHeaders(r.Header, header) - if sp := r.Header.Get("Sec-WebSocket-Protocol"); sp != "" { header.Set("Sec-WebSocket-Protocol", sp) } return header } -// copyClientHeaders copies allowed headers from client request -func (_ *Handler) copyClientHeaders(src, dst http.Header) { - excludedHeaders := map[string]bool{ - "authorization": true, - "host": true, - "connection": true, - "upgrade": true, - "sec-websocket-key": true, - "sec-websocket-version": true, - "sec-websocket-extensions": true, - "origin": true, - "sec-websocket-protocol": true, - "x-user-email": true, - "x-request-id": true, - "x-audit-resource": true, - } - - for k, vals := range src { - if excludedHeaders[strings.ToLower(k)] { - continue - } - for _, v := range vals { - dst.Add(k, v) - } - } -} - // dialUpstreamWebSocket establishes connection to upstream WebSocket func (h *Handler) dialUpstreamWebSocket( ctx context.Context, @@ -268,6 +231,7 @@ func (h *Handler) dialWithRedirects( var resp *http.Response var err error + rackHost := wsURL.Host for i := 0; i < 3; i++ { conn, resp, err = dialer.Dial(wsURL.String(), header) if err == nil { @@ -282,6 +246,10 @@ func (h *Handler) dialWithRedirects( if parseErr != nil { break } + // The dial carries the rack credential; never send it anywhere but the configured rack. + if !strings.EqualFold(newURL.Host, rackHost) { + return nil, nil, fmt.Errorf("refusing websocket redirect from %s to %s", rackHost, newURL.Host) + } wsURL = newURL } diff --git a/internal/gateway/rbac/action_string.go b/internal/gateway/rbac/action_string.go index 39aeaaba..56591843 100644 --- a/internal/gateway/rbac/action_string.go +++ b/internal/gateway/rbac/action_string.go @@ -30,11 +30,12 @@ func _() { _ = x[ActionUnset-19] _ = x[ActionUpdate-20] _ = x[ActionUpdateName-21] + _ = x[ActionRunPrivileged-22] } -const _Action_name = "addapprovecreatedeletedeploy_with_approvalexecgenerateimportkeyrolllistmanagepromotereadremoverestartsetstartstopterminateunsetupdateupdate_name" +const _Action_name = "addapprovecreatedeletedeploy_with_approvalexecgenerateimportkeyrolllistmanagepromotereadremoverestartsetstartstopterminateunsetupdateupdate_namerun_privileged" -var _Action_index = [...]uint8{0, 3, 10, 16, 22, 42, 46, 54, 60, 67, 71, 77, 84, 88, 94, 101, 104, 109, 113, 122, 127, 133, 144} +var _Action_index = [...]uint8{0, 3, 10, 16, 22, 42, 46, 54, 60, 67, 71, 77, 84, 88, 94, 101, 104, 109, 113, 122, 127, 133, 144, 158} func (i Action) String() string { idx := int(i) - 0 diff --git a/internal/gateway/rbac/constants.go b/internal/gateway/rbac/constants.go index 9b39546b..85215850 100644 --- a/internal/gateway/rbac/constants.go +++ b/internal/gateway/rbac/constants.go @@ -245,15 +245,18 @@ const ( ActionUpdate // update // ActionUpdateName represents updating only the resource name. ActionUpdateName // update_name + // ActionRunPrivileged represents starting a process with options that escape the app's release + // image or scheduling constraints (custom image, host volumes, privileged mode, node placement). + ActionRunPrivileged // run_privileged ) // IsValid reports whether the action represents a defined value. -func (a Action) IsValid() bool { return a <= ActionUpdateName } +func (a Action) IsValid() bool { return a <= ActionRunPrivileged } // ParseAction converts a string name into an Action value. func ParseAction(v string) (Action, error) { // Try each known value - for a := ActionAdd; a <= ActionUpdateName; a++ { + for a := ActionAdd; a <= ActionRunPrivileged; a++ { if a.String() == v { return a, nil } diff --git a/internal/gateway/routes/setup_helpers.go b/internal/gateway/routes/setup_helpers.go index 870e89b5..9b74c429 100644 --- a/internal/gateway/routes/setup_helpers.go +++ b/internal/gateway/routes/setup_helpers.go @@ -13,6 +13,8 @@ import ( ) func setupGlobalMiddleware(router *gin.Engine, cfg *Config) { + router.Use(middleware.StripInternalHeaders()) + if cfg.SentryEnabled { options := sentrygin.Options{ Repanic: true, From 2d97393d673dcc85f2aba4126b5ac0de042c986a Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Fri, 9 Oct 2026 15:32:57 +1300 Subject: [PATCH 05/28] Scrub credentials from Sentry events; refuse test-only switches in production Sentry was initialised with SendDefaultPII=true, so every 5xx event carried the Authorization header (session or API token plus inline MFA code) and session cookies. PII collection is now off, and a BeforeSend hook strips everything except a small header allowlist, plus cookies, query strings, request bodies and env. The gateway now refuses to start when its database is marked production while any test-only switch is set: DEV_MODE, E2E_TEST_MODE (skips WebAuthn assertion checks), AWS_ENDPOINT_URL_S3 or POSTMARK_API_BASE. GOOGLE_OAUTH_BASE_URL must be https there. Development and E2E databases are unaffected. --- internal/gateway/app/app.go | 9 +++ internal/gateway/app/production_guard.go | 45 +++++++++++++++ internal/gateway/app/production_guard_test.go | 55 +++++++++++++++++++ internal/gateway/app/sentry.go | 31 ++++++++++- internal/gateway/app/sentry_test.go | 33 +++++++++++ 5 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 internal/gateway/app/production_guard.go create mode 100644 internal/gateway/app/production_guard_test.go diff --git a/internal/gateway/app/app.go b/internal/gateway/app/app.go index b6b16be9..179a1a4d 100644 --- a/internal/gateway/app/app.go +++ b/internal/gateway/app/app.go @@ -2,6 +2,7 @@ package app import ( "context" + "os" "time" "github.com/gin-gonic/gin" @@ -40,6 +41,14 @@ func New() (*App, error) { database.Close() //nolint:errcheck,gosec // G104: cleanup on init failure return nil, err } + dbEnvironment, err := database.CurrentEnvironment() + if err == nil { + err = checkProductionSafety(dbEnvironment, os.Getenv) + } + if err != nil { + database.Close() //nolint:errcheck,gosec // G104: cleanup on init failure + return nil, err + } // Initialize dependencies app := &App{ diff --git a/internal/gateway/app/production_guard.go b/internal/gateway/app/production_guard.go new file mode 100644 index 00000000..baa0b429 --- /dev/null +++ b/internal/gateway/app/production_guard.go @@ -0,0 +1,45 @@ +package app + +import ( + "fmt" + "strconv" + "strings" +) + +// testOnlyFlags and testOnlyEndpoints switch off or redirect security controls for the development and +// E2E stacks (E2E_TEST_MODE skips WebAuthn assertion checks, DEV_MODE relaxes cookies/CSP/secrets, and the +// endpoint overrides send audit anchors and the Postmark token elsewhere). They must never be active +// against a database marked as production. +var ( + testOnlyFlags = []string{"DEV_MODE", "E2E_TEST_MODE"} + testOnlyEndpoints = []string{"AWS_ENDPOINT_URL_S3", "POSTMARK_API_BASE"} +) + +// checkProductionSafety refuses to start a gateway whose database is marked production while any +// test-only switch is set. getenv is os.Getenv in production; tests pass a fake. +func checkProductionSafety(dbEnvironment string, getenv func(string) string) error { + if dbEnvironment != "production" { + return nil + } + var offending []string + for _, name := range testOnlyFlags { + if enabled, err := strconv.ParseBool(strings.TrimSpace(getenv(name))); err == nil && enabled { + offending = append(offending, name) + } + } + for _, name := range testOnlyEndpoints { + if strings.TrimSpace(getenv(name)) != "" { + offending = append(offending, name) + } + } + if base := strings.TrimSpace(getenv("GOOGLE_OAUTH_BASE_URL")); base != "" && !strings.HasPrefix(base, "https://") { + offending = append(offending, "GOOGLE_OAUTH_BASE_URL (must be https)") + } + if len(offending) > 0 { + return fmt.Errorf( + "refusing to start: the database is marked production but test-only settings are set: %s", + strings.Join(offending, ", "), + ) + } + return nil +} diff --git a/internal/gateway/app/production_guard_test.go b/internal/gateway/app/production_guard_test.go new file mode 100644 index 00000000..34536c61 --- /dev/null +++ b/internal/gateway/app/production_guard_test.go @@ -0,0 +1,55 @@ +package app + +import "testing" + +func TestCheckProductionSafety(t *testing.T) { + env := func(values map[string]string) func(string) string { + return func(name string) string { return values[name] } + } + cases := []struct { + name string + dbEnv string + vars map[string]string + wantErr bool + }{ + {"production with nothing set", "production", nil, false}, + { + "production with flags explicitly false", "production", + map[string]string{"DEV_MODE": "false", "E2E_TEST_MODE": "0"}, + false, + }, + { + "production with https OIDC issuer", "production", + map[string]string{"GOOGLE_OAUTH_BASE_URL": "https://accounts.example.com"}, + false, + }, + {"production with E2E_TEST_MODE", "production", map[string]string{"E2E_TEST_MODE": "true"}, true}, + {"production with DEV_MODE", "production", map[string]string{"DEV_MODE": "1"}, true}, + { + "production with S3 endpoint override", "production", + map[string]string{"AWS_ENDPOINT_URL_S3": "http://minio:9000"}, + true, + }, + { + "production with Postmark override", "production", + map[string]string{"POSTMARK_API_BASE": "http://evil"}, + true, + }, + { + "production with http OIDC issuer", "production", + map[string]string{"GOOGLE_OAUTH_BASE_URL": "http://mock-oauth:3345"}, + true, + }, + { + "development allows test switches", "development", + map[string]string{"DEV_MODE": "true", "E2E_TEST_MODE": "true", "AWS_ENDPOINT_URL_S3": "x"}, + false, + }, + } + for _, tc := range cases { + err := checkProductionSafety(tc.dbEnv, env(tc.vars)) + if (err != nil) != tc.wantErr { + t.Errorf("%s: err=%v, wantErr=%v", tc.name, err, tc.wantErr) + } + } +} diff --git a/internal/gateway/app/sentry.go b/internal/gateway/app/sentry.go index 04bdb9c4..fdcf57f9 100644 --- a/internal/gateway/app/sentry.go +++ b/internal/gateway/app/sentry.go @@ -23,7 +23,10 @@ func buildSentryOptions(cfg *config.Config) (sentry.ClientOptions, bool) { } opts.Dsn = dsn opts.AttachStacktrace = true - opts.SendDefaultPII = true + // Requests carry session tokens, API tokens and inline MFA codes in headers and cookies. + // Never let the SDK attach them, and scrub anything that gets through. + opts.SendDefaultPII = false + opts.BeforeSend = scrubSentryEvent env := strings.TrimSpace(cfg.SentryEnvironment) if env == "" { @@ -60,3 +63,29 @@ func initializeSentry(cfg *config.Config) (bool, error) { log.Printf("Sentry enabled (environment=%s, release=%s)", opts.Environment, opts.Release) return true, nil } + +// sentryHeaderAllowlist lists the only request headers forwarded to Sentry. +var sentryHeaderAllowlist = map[string]bool{ + "accept": true, + "content-type": true, + "user-agent": true, + "x-request-id": true, +} + +// scrubSentryEvent removes credentials from request data before an event leaves the gateway. +func scrubSentryEvent(event *sentry.Event, _ *sentry.EventHint) *sentry.Event { + if event == nil || event.Request == nil { + return event + } + req := event.Request + for name := range req.Headers { + if !sentryHeaderAllowlist[strings.ToLower(name)] { + delete(req.Headers, name) + } + } + req.Cookies = "" + req.QueryString = "" + req.Data = "" + req.Env = nil + return event +} diff --git a/internal/gateway/app/sentry_test.go b/internal/gateway/app/sentry_test.go index 955e1bcf..41126103 100644 --- a/internal/gateway/app/sentry_test.go +++ b/internal/gateway/app/sentry_test.go @@ -3,6 +3,8 @@ package app import ( "testing" + "github.com/getsentry/sentry-go" + "github.com/DocSpring/rack-gateway/internal/gateway/config" ) @@ -53,3 +55,34 @@ func TestBuildSentryOptionsDevelopmentEnvironment(t *testing.T) { t.Fatalf("expected environment to default to development in dev mode, got %q", opts.Environment) } } + +func TestScrubSentryEventRemovesCredentials(t *testing.T) { + opts, enabled := buildSentryOptions(&config.Config{SentryDSN: "https://key@o0.ingest.sentry.io/1"}) + if !enabled || opts.SendDefaultPII || opts.BeforeSend == nil { + t.Fatalf("expected PII disabled and a scrubber, got SendDefaultPII=%v BeforeSend=%v", + opts.SendDefaultPII, opts.BeforeSend != nil) + } + + event := &sentry.Event{Request: &sentry.Request{ + Headers: map[string]string{ + "Authorization": "Basic Y29udm94OnNlc3Npb24udG90cC4xMjM0NTY=", + "Cookie": "session_token=abc", + "X-CSRF-Token": "csrf", + "X-MFA-TOTP": "123456", + "User-Agent": "rack-gateway/1.0", + }, + Cookies: "session_token=abc", + QueryString: "state=secret&code=oauth", + Data: `{"set":{"SECRET":"x"}}`, + Env: map[string]string{"REMOTE_ADDR": "10.0.0.1"}, + }} + scrubbed := opts.BeforeSend(event, nil) + + if got := scrubbed.Request.Headers; len(got) != 1 || got["User-Agent"] != "rack-gateway/1.0" { + t.Fatalf("expected only User-Agent to survive, got %v", got) + } + if scrubbed.Request.Cookies != "" || scrubbed.Request.QueryString != "" || scrubbed.Request.Data != "" || + scrubbed.Request.Env != nil { + t.Fatalf("expected cookies, query string, body and env to be cleared: %+v", scrubbed.Request) + } +} From c7583f8f3881fad3adad3596472d2ee4096534e3 Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Fri, 9 Oct 2026 15:39:57 +1300 Subject: [PATCH 06/28] ci: pin actions by SHA, verify installer checksums, least-privilege tokens - Pin every third-party action to a full commit SHA (newest release within the current major), with the version in a comment. - Install Task, the Convox CLI and TruffleHog from pinned releases with SHA-256 verification instead of piping install scripts (including golangci-lint's install.sh from master) into sh. The golangci-lint action already installs the pinned version and verifies the config schema, so the separate install and config-verify steps are gone. govulncheck is pinned to v1.8.0. - ci.yml and e2e.yml run with contents: read. The docs workflow gives pages/id-token write only to the deploy job, pins Bun and installs with --frozen-lockfile. - New daily Security Scan workflow: govulncheck, bun audit (high+) for web, and a Trivy scan of the published image, so new advisories surface without a push (CI was red for a month unnoticed). - Dependabot for gomod, bun (web, docs, mock-oauth), GitHub Actions and Dockerfiles, weekly with grouped minor/patch updates. --- .github/actions/install-convox/action.yml | 28 ++++++++ .github/actions/install-task/action.yml | 29 +++++++++ .github/dependabot.yml | 43 ++++++++++++ .github/workflows/ci.yml | 79 ++++++++++------------- .github/workflows/docs.yml | 21 +++--- .github/workflows/e2e.yml | 53 ++++++--------- .github/workflows/security-scan.yml | 68 +++++++++++++++++++ scripts/build-and-push.sh | 35 ---------- 8 files changed, 235 insertions(+), 121 deletions(-) create mode 100644 .github/actions/install-convox/action.yml create mode 100644 .github/actions/install-task/action.yml create mode 100644 .github/dependabot.yml create mode 100644 .github/workflows/security-scan.yml delete mode 100755 scripts/build-and-push.sh diff --git a/.github/actions/install-convox/action.yml b/.github/actions/install-convox/action.yml new file mode 100644 index 00000000..5906c131 --- /dev/null +++ b/.github/actions/install-convox/action.yml @@ -0,0 +1,28 @@ +name: Install Convox CLI +description: Install a pinned, checksum-verified Convox CLI (linux amd64) into /usr/local/bin. + +inputs: + version: + description: Convox CLI release version + default: "3.25.7" + sha256: + description: SHA-256 of the convox-linux asset for that version + default: 6a0ffe6faf269302c311c2f8e4d1cdc116902c933c7d91f461c47d8a25190759 + +runs: + using: composite + steps: + - name: Install Convox CLI + shell: bash + env: + CONVOX_VERSION: ${{ inputs.version }} + CONVOX_SHA256: ${{ inputs.sha256 }} + run: | + set -euo pipefail + tmp="$(mktemp -d)" + curl -fsSL -o "${tmp}/convox" \ + "https://github.com/convox/convox/releases/download/${CONVOX_VERSION}/convox-linux" + echo "${CONVOX_SHA256} ${tmp}/convox" | sha256sum --check --strict + sudo install -m 0755 "${tmp}/convox" /usr/local/bin/convox + rm -rf "${tmp}" + convox version || true diff --git a/.github/actions/install-task/action.yml b/.github/actions/install-task/action.yml new file mode 100644 index 00000000..47868220 --- /dev/null +++ b/.github/actions/install-task/action.yml @@ -0,0 +1,29 @@ +name: Install Task +description: Install a pinned, checksum-verified Task (taskfile.dev) binary into /usr/local/bin. + +inputs: + version: + description: Task release version + default: "3.53.1" + sha256: + description: SHA-256 of task_linux_amd64.tar.gz for that version (from task_checksums.txt) + default: a54a408f6861ff921f6e87774180db31bacd8c1e7c944ca696db9fea49a82fc7 + +runs: + using: composite + steps: + - name: Install Task + shell: bash + env: + TASK_VERSION: ${{ inputs.version }} + TASK_SHA256: ${{ inputs.sha256 }} + run: | + set -euo pipefail + tmp="$(mktemp -d)" + curl -fsSL -o "${tmp}/task.tar.gz" \ + "https://github.com/go-task/task/releases/download/v${TASK_VERSION}/task_linux_amd64.tar.gz" + echo "${TASK_SHA256} ${tmp}/task.tar.gz" | sha256sum --check --strict + tar -xzf "${tmp}/task.tar.gz" -C "${tmp}" task + sudo install -m 0755 "${tmp}/task" /usr/local/bin/task + rm -rf "${tmp}" + task --version diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..4dcc6101 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,43 @@ +version: 2 + +updates: + - package-ecosystem: gomod + directory: / + schedule: + interval: weekly + ignore: + # Replaced by the local modules in internal/shims (see go.mod replace directives) + - dependency-name: github.com/docker/docker + - dependency-name: github.com/moby/buildkit + groups: + go-minor-patch: + update-types: [minor, patch] + + - package-ecosystem: bun + directories: + - /web + - /docs + - /mock-oauth + schedule: + interval: weekly + groups: + bun-minor-patch: + update-types: [minor, patch] + + - package-ecosystem: github-actions + directory: / + schedule: + interval: weekly + groups: + actions-minor-patch: + update-types: [minor, patch] + + - package-ecosystem: docker + directories: + - / + - /mock-oauth + schedule: + interval: weekly + groups: + docker-minor-patch: + update-types: [minor, patch] diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 557e4917..59195aac 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,9 @@ on: pull_request: branches: [main] +permissions: + contents: read + jobs: go-tests: runs-on: ubuntu-latest @@ -14,10 +17,10 @@ jobs: TEST_DATABASE_URL: postgres://postgres:postgres@localhost:55432/gateway_test?sslmode=disable GOLANGCI_LINT_VERSION: v2.11.1 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Setup Go - uses: actions/setup-go@v5 + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: go-version: "1.26.9" @@ -27,25 +30,13 @@ jobs: sudo apt-get install -y libfido2-dev libudev-dev pkg-config - name: Install Task - run: | - curl -sL https://taskfile.dev/install.sh | sh -s -- -b /usr/local/bin - task --version + uses: ./.github/actions/install-task - name: Go deps run: task go:deps - name: Install Convox CLI - run: | - set -euo pipefail - ARCH=$(uname -m) - URL="https://github.com/convox/convox/releases/latest/download/convox-linux" - if [ "$ARCH" = "aarch64" ] || [ "$ARCH" = "arm64" ]; then - URL="https://github.com/convox/convox/releases/latest/download/convox-linux-arm64" - fi - curl -fsSL "$URL" -o /tmp/convox - sudo mv /tmp/convox /usr/local/bin/convox - sudo chmod 755 /usr/local/bin/convox - convox version || true + uses: ./.github/actions/install-convox - name: Install Go tools run: task go:tools @@ -58,15 +49,15 @@ jobs: env: GOLANGCI_LINT_VERSION: v2.11.1 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Setup Go - uses: actions/setup-go@v5 + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: go-version: "1.26.9" - name: Cache golangci-lint cache - uses: actions/cache@v4 + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 with: path: | ~/.cache/golangci-lint @@ -78,24 +69,15 @@ jobs: sudo apt-get install -y libfido2-dev libudev-dev pkg-config - name: Install Task - run: | - curl -sL https://taskfile.dev/install.sh | sh -s -- -b /usr/local/bin - task --version - - - name: Install golangci-lint - run: | - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh \ - | sudo sh -s -- -b /usr/local/bin "${GOLANGCI_LINT_VERSION}" - golangci-lint version + uses: ./.github/actions/install-task - name: Go deps (lint warmup) run: task go:deps - - name: Verify golangci-lint config - run: task go:lint:config - + # The action installs the pinned golangci-lint release and verifies .golangci.yml + # against its JSON schema (verify: true) before linting. - name: golangci-lint - uses: golangci/golangci-lint-action@v8 + uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8.0.0 with: version: ${{ env.GOLANGCI_LINT_VERSION }} env: @@ -121,14 +103,25 @@ jobs: run: task shellcheck - name: Install govulncheck - run: go install golang.org/x/vuln/cmd/govulncheck@latest + run: go install golang.org/x/vuln/cmd/govulncheck@v1.8.0 - name: Check for vulnerabilities run: task go:sec:vuln - name: Install TruffleHog + env: + TRUFFLEHOG_VERSION: "3.97.9" + TRUFFLEHOG_SHA256: 40377e6572495412fb9ba0bc21c9401f73b72f1d2afd11b9931bc4a5ed622866 run: | - curl -sSfL https://raw.githubusercontent.com/trufflesecurity/trufflehog/main/scripts/install.sh | sh -s -- -b /usr/local/bin + set -euo pipefail + tmp="$(mktemp -d)" + curl -fsSL -o "${tmp}/trufflehog.tar.gz" \ + "https://github.com/trufflesecurity/trufflehog/releases/download/v${TRUFFLEHOG_VERSION}/trufflehog_${TRUFFLEHOG_VERSION}_linux_amd64.tar.gz" + echo "${TRUFFLEHOG_SHA256} ${tmp}/trufflehog.tar.gz" | sha256sum --check --strict + tar -xzf "${tmp}/trufflehog.tar.gz" -C "${tmp}" trufflehog + sudo install -m 0755 "${tmp}/trufflehog" /usr/local/bin/trufflehog + rm -rf "${tmp}" + trufflehog --version - name: Scan for secrets run: task go:sec:secrets @@ -136,15 +129,15 @@ jobs: web-tests: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Setup Node - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: "20" - name: Setup Bun - uses: oven-sh/setup-bun@v2 + uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: bun-version: "1.3.1" @@ -154,9 +147,7 @@ jobs: bun install --frozen-lockfile - name: Install Task - run: | - curl -sL https://taskfile.dev/install.sh | sh -s -- -b /usr/local/bin - task --version + uses: ./.github/actions/install-task - name: Web lint (Typecheck, Biome, and knip) run: task web:lint @@ -170,17 +161,15 @@ jobs: mock-oauth-tests: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Setup Bun - uses: oven-sh/setup-bun@v2 + uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: bun-version: "1.3.1" - name: Install Task - run: | - curl -sL https://taskfile.dev/install.sh | sh -s -- -b /usr/local/bin - task --version + uses: ./.github/actions/install-task - name: Mock OAuth lint (Typecheck and Biome) run: task mock-oauth:lint diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 540e6968..f36da583 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -6,11 +6,8 @@ on: paths: ["docs/**"] workflow_dispatch: -# Sets permissions of the GITHUB_TOKEN to allow deployment to GitHub Pages permissions: contents: read - pages: write - id-token: write # Allow only one concurrent deployment, skipping runs queued between the run in-progress and latest queued. # However, do NOT cancel in-progress runs as we want to allow these production deployments to complete. @@ -23,32 +20,38 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Setup Bun - uses: oven-sh/setup-bun@v2 + uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 + with: + bun-version: "1.3.1" - name: Install dependencies - run: cd docs && bun install + run: cd docs && bun install --frozen-lockfile - name: Build docs run: cd docs && bun run build - name: Setup Pages - uses: actions/configure-pages@v5 + uses: actions/configure-pages@983d7736d9b0ae728b81ab479565c72886d7745b # v5.0.0 - name: Upload artifact - uses: actions/upload-pages-artifact@v3 + uses: actions/upload-pages-artifact@56afc609e74202658d3ffba0e8f6dda462b719fa # v3.0.1 with: path: docs/dist deploy: needs: build runs-on: ubuntu-latest + # Only the deploy job may publish to GitHub Pages + permissions: + pages: write + id-token: write environment: name: github-pages url: ${{ steps.deployment.outputs.page_url }} steps: - name: Deploy to GitHub Pages id: deployment - uses: actions/deploy-pages@v4 + uses: actions/deploy-pages@d6db90164ac5ed86f2b6aed7e0febac5b3c0c03e # v4.0.5 diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index e1912131..fbad9928 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -5,6 +5,9 @@ on: branches: [main] pull_request: +permissions: + contents: read + jobs: build-image: runs-on: ubuntu-latest @@ -12,13 +15,13 @@ jobs: GATEWAY_IMAGE: rack-gateway-api:e2e-${{ github.sha }} steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3 + uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3.12.0 - name: Build gateway image (with web UI) and export - uses: docker/build-push-action@v6 + uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6.19.2 with: context: . file: Dockerfile @@ -31,7 +34,7 @@ jobs: outputs: type=docker,dest=${{ runner.temp }}/gateway-api.tar - name: Upload image artifact - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: name: gateway-image path: ${{ runner.temp }}/gateway-api.tar @@ -54,12 +57,10 @@ jobs: CGO_ENABLED: 1 steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Install Task - run: | - curl -sL https://taskfile.dev/install.sh | sh -s -- -b /usr/local/bin - task --version + uses: ./.github/actions/install-task - name: Install libfido2 dependencies run: | @@ -67,24 +68,24 @@ jobs: sudo apt-get install -y libfido2-dev libudev-dev pkg-config - name: Setup Node - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: "20" - name: Setup Bun - uses: oven-sh/setup-bun@v2 + uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: bun-version: "1.3.1" - name: Download gateway image - uses: actions/download-artifact@v4 + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 with: name: gateway-image path: ${{ runner.temp }} - name: Load gateway image run: | - docker load --input ${{ runner.temp }}/gateway-api.tar + docker load --input "${RUNNER_TEMP}/gateway-api.tar" docker image ls -a | grep rack-gateway-api || true - name: Build mock services @@ -108,8 +109,8 @@ jobs: run: | set -x docker compose ps - curl -sv http://127.0.0.1:${GATEWAY_PORT}/api/v1/health || true - curl -sv http://127.0.0.1:${GATEWAY_PORT}/app/login -o /dev/null || true + curl -sv "http://127.0.0.1:${GATEWAY_PORT}/api/v1/health" || true + curl -sv "http://127.0.0.1:${GATEWAY_PORT}/app/login" -o /dev/null || true - name: Install Playwright and deps working-directory: web @@ -142,12 +143,10 @@ jobs: CGO_ENABLED: 1 steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Install Task - run: | - curl -sL https://taskfile.dev/install.sh | sh -s -- -b /usr/local/bin - task --version + uses: ./.github/actions/install-task - name: Install libfido2 dependencies run: | @@ -155,19 +154,19 @@ jobs: sudo apt-get install -y libfido2-dev libudev-dev pkg-config - name: Setup Go - uses: actions/setup-go@v5 + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: go-version: "1.26.9" - name: Download gateway image - uses: actions/download-artifact@v4 + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 with: name: gateway-image path: ${{ runner.temp }} - name: Load gateway image run: | - docker load --input ${{ runner.temp }}/gateway-api.tar + docker load --input "${RUNNER_TEMP}/gateway-api.tar" docker image ls -a | grep rack-gateway-api || true - name: Build mock services @@ -187,17 +186,7 @@ jobs: GATEWAY_PORT=9447 WEB_PORT=9447 MOCK_OAUTH_PORT=9345 CHECK_VITE_PROXY=false ./scripts/wait-for-services.sh - name: Install Convox CLI - run: | - set -euo pipefail - ARCH=$(uname -m) - URL="https://github.com/convox/convox/releases/latest/download/convox-linux" - if [ "$ARCH" = "aarch64" ] || [ "$ARCH" = "arm64" ]; then - URL="https://github.com/convox/convox/releases/latest/download/convox-linux-arm64" - fi - curl -fsSL "$URL" -o /tmp/convox - sudo mv /tmp/convox /usr/local/bin/convox - sudo chmod 755 /usr/local/bin/convox - convox version || true + uses: ./.github/actions/install-convox - name: Run CLI E2E run: GATEWAY_PORT=9447 MOCK_OAUTH_PORT=9345 MOCK_CONVOX_PORT=6443 E2E_DATABASE_NAME=gateway_test E2E_GATEWAY_SERVICE=gateway-api-test ./scripts/cli-e2e.sh diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml new file mode 100644 index 00000000..82d6e165 --- /dev/null +++ b/.github/workflows/security-scan.yml @@ -0,0 +1,68 @@ +name: Security Scan + +# Daily scan so newly disclosed vulnerabilities show up even when nobody pushes. +# GitHub emails the workflow's last editor when a scheduled run fails. +on: + schedule: + - cron: "17 18 * * *" # daily, 06:17 NZST + workflow_dispatch: {} + +permissions: + contents: read + +jobs: + govulncheck: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + persist-credentials: false + + - name: Setup Go + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 + with: + go-version: "1.26.9" + cache: false + + - name: Install libfido2 dependencies + run: | + sudo apt-get update + sudo apt-get install -y libfido2-dev libudev-dev pkg-config + + - name: Install govulncheck + run: go install golang.org/x/vuln/cmd/govulncheck@v1.8.0 + + - name: Check Go vulnerabilities + run: govulncheck ./... + + web-audit: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + persist-credentials: false + + - name: Setup Bun + uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 + with: + bun-version: "1.3.1" + + - name: Install web packages + working-directory: web + run: bun install --frozen-lockfile + + - name: Audit web dependencies (high and critical) + working-directory: web + run: bun audit --audit-level=high + + image-scan: + runs-on: ubuntu-latest + steps: + - name: Scan the published gateway image + uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 + with: + version: v0.74.0 + image-ref: docker.io/docspringcom/rack-gateway:latest + severity: HIGH,CRITICAL + ignore-unfixed: true + exit-code: "1" diff --git a/scripts/build-and-push.sh b/scripts/build-and-push.sh deleted file mode 100755 index 46c59371..00000000 --- a/scripts/build-and-push.sh +++ /dev/null @@ -1,35 +0,0 @@ -#!/bin/bash -set -e - -# Docker Hub image name -IMAGE_NAME="docker.io/docspringcom/rack-gateway" - -# Get commit SHA from git -COMMIT_SHA=$(git rev-parse --short HEAD) - -echo "Building rack-gateway for Docker Hub..." -echo "Image: ${IMAGE_NAME}" -echo "Commit: ${COMMIT_SHA}" - -# Build for amd64 (linux/amd64) -# Use DOCKER_DEFAULT_PLATFORM to ensure amd64 build -DOCKER_DEFAULT_PLATFORM=linux/amd64 docker build \ - --tag "${IMAGE_NAME}:${COMMIT_SHA}" \ - --tag "${IMAGE_NAME}:latest" \ - . - -echo "" -echo "Build complete! Images tagged:" -echo " - ${IMAGE_NAME}:${COMMIT_SHA}" -echo " - ${IMAGE_NAME}:latest" - -echo "" -echo "Pushing to Docker Hub..." -docker push "${IMAGE_NAME}:${COMMIT_SHA}" -docker push "${IMAGE_NAME}:latest" - -echo "" -echo "Push complete!" -echo "" -echo "To deploy, update convox.yml to use: image: ${IMAGE_NAME}:${COMMIT_SHA}" -echo "Then run: convox deploy" From 0a669fe640cb7f6c18487faaa86fb829ccc32fda Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Fri, 9 Oct 2026 15:40:05 +1300 Subject: [PATCH 07/28] docker: pin base images by digest, run the gateway as non-root, drop curl - Pin oven/bun, golang and alpine base images by digest (runtime moves from alpine:latest to alpine:3.24.2) in all Dockerfiles, including mock-oauth's floating oven/bun:1-alpine. - The production image runs as UID/GID 10001. Binaries, web assets and start-gateway.sh stay root-owned and read-only; the gateway only writes temp files under /tmp. - curl is no longer installed in the gateway images. The compose healthcheck uses busybox wget, which is already in alpine. - Fix the commit hash in the gateway binary: the build stage read an unset COMMIT_HASH arg, so every image reported CommitHash=unknown. It now uses the required COMMIT_SHA build arg that CI already passes. --- Dockerfile | 24 +++++++++++++++--------- Dockerfile.gateway-dev | 6 +++--- Dockerfile.mock-convox | 4 ++-- docker-compose.yml | 2 +- mock-oauth/Dockerfile | 2 +- 5 files changed, 22 insertions(+), 16 deletions(-) diff --git a/Dockerfile b/Dockerfile index efe2d0d5..01fad852 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # syntax=docker/dockerfile:1 -FROM oven/bun:1.3.2-alpine AS webbuild +FROM oven/bun:1.3.2-alpine@sha256:adda30fd4db7d8ef9a2113cb935c6f751de3daad39373713b56eefe49db78471 AS webbuild ARG COMMIT_SHA RUN test -n "$COMMIT_SHA" || (echo "COMMIT_SHA build arg is required" && exit 1) @@ -15,7 +15,7 @@ RUN bun install --frozen-lockfile COPY web/ ./ RUN bun run build -FROM golang:1.26.9-alpine AS builder +FROM golang:1.26.9-alpine@sha256:cdfd4fe2da6b225d8b40c6b7a105736e548e83ff56d5d8f9394446eeb5eb84e0 AS builder RUN apk add --no-cache git ca-certificates make gcc musl-dev nodejs npm @@ -33,24 +33,30 @@ COPY web/package.json ./web/package.json COPY internal ./internal COPY cmd/gateway ./cmd/gateway -# Build the gateway binary with version info -ARG COMMIT_HASH=unknown +# Build the gateway binary with version info (COMMIT_SHA is the same build arg the web stage requires) +ARG COMMIT_SHA +RUN test -n "$COMMIT_SHA" || (echo "COMMIT_SHA build arg is required" && exit 1) RUN VERSION=$(node -p "require('./web/package.json').version") && \ CGO_ENABLED=0 go build \ - -ldflags "-X github.com/DocSpring/rack-gateway/internal/gateway/version.Version=${VERSION} -X github.com/DocSpring/rack-gateway/internal/gateway/version.CommitHash=${COMMIT_HASH}" \ + -ldflags "-X github.com/DocSpring/rack-gateway/internal/gateway/version.Version=${VERSION} -X github.com/DocSpring/rack-gateway/internal/gateway/version.CommitHash=${COMMIT_SHA}" \ -o /out/rack-gateway-api ./cmd/gateway \ && /out/rack-gateway-api help -FROM alpine:latest +FROM alpine:3.24.2@sha256:294b683cb724975bec92580e1e685676bd4b50bda910ddb8c51d4cabeaec77e6 -RUN apk --no-cache add ca-certificates curl +# ca-certificates for outbound TLS. No curl: the compose healthcheck uses busybox wget. +RUN apk --no-cache add ca-certificates \ + && addgroup -S -g 10001 gateway \ + && adduser -S -D -H -u 10001 -G gateway -s /sbin/nologin gateway WORKDIR /app +# Files stay root-owned and read-only to the runtime user. COPY --from=builder /out/rack-gateway-api ./ COPY --from=webbuild /app/web/dist ./web/dist -COPY scripts/start-gateway.sh ./scripts/start-gateway.sh -RUN chmod +x ./scripts/start-gateway.sh +COPY --chmod=0755 scripts/start-gateway.sh ./scripts/start-gateway.sh + +USER 10001:10001 EXPOSE 8080 diff --git a/Dockerfile.gateway-dev b/Dockerfile.gateway-dev index 963b6a02..db7737a2 100644 --- a/Dockerfile.gateway-dev +++ b/Dockerfile.gateway-dev @@ -1,4 +1,4 @@ -FROM golang:1.26.9-alpine AS builder +FROM golang:1.26.9-alpine@sha256:cdfd4fe2da6b225d8b40c6b7a105736e548e83ff56d5d8f9394446eeb5eb84e0 AS builder RUN apk add --no-cache git ca-certificates make gcc musl-dev @@ -21,9 +21,9 @@ RUN --mount=type=cache,target=/go/pkg/mod,sharing=locked \ CGO_ENABLED=0 go build -o /out/rack-gateway-api ./cmd/gateway \ && /out/rack-gateway-api help -FROM alpine:latest +FROM alpine:3.24.2@sha256:294b683cb724975bec92580e1e685676bd4b50bda910ddb8c51d4cabeaec77e6 -RUN apk --no-cache add ca-certificates curl +RUN apk --no-cache add ca-certificates WORKDIR /root/ diff --git a/Dockerfile.mock-convox b/Dockerfile.mock-convox index 0831c183..27d79607 100644 --- a/Dockerfile.mock-convox +++ b/Dockerfile.mock-convox @@ -1,7 +1,7 @@ # Build stage # syntax=docker/dockerfile:1.5 -FROM golang:1.26.9-alpine AS builder +FROM golang:1.26.9-alpine@sha256:cdfd4fe2da6b225d8b40c6b7a105736e548e83ff56d5d8f9394446eeb5eb84e0 AS builder RUN apk add --no-cache git make @@ -25,7 +25,7 @@ RUN --mount=type=cache,target=/go/pkg/mod,sharing=locked \ && /out/mock-convox help # Final stage -FROM alpine:latest +FROM alpine:3.24.2@sha256:294b683cb724975bec92580e1e685676bd4b50bda910ddb8c51d4cabeaec77e6 RUN apk --no-cache add ca-certificates diff --git a/docker-compose.yml b/docker-compose.yml index f8d32e9a..d67f8958 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -18,7 +18,7 @@ x-gateway-env: &gateway_common_env x-gateway-health: &gateway_healthcheck test: - ["CMD-SHELL", "curl -fsS http://localhost:$$PORT/api/v1/health || exit 1"] + ["CMD-SHELL", "wget -q -O /dev/null http://localhost:$$PORT/api/v1/health || exit 1"] interval: 2s timeout: 5s retries: 3 diff --git a/mock-oauth/Dockerfile b/mock-oauth/Dockerfile index 0f56aa66..7076e3fa 100644 --- a/mock-oauth/Dockerfile +++ b/mock-oauth/Dockerfile @@ -1,6 +1,6 @@ # syntax=docker/dockerfile:1.6 -FROM oven/bun:1-alpine AS base +FROM oven/bun:1.3.2-alpine@sha256:adda30fd4db7d8ef9a2113cb935c6f751de3daad39373713b56eefe49db78471 AS base RUN apk add --no-cache curl From 49cc881ff5a585a0d1d3bf42530ba52c0d76f650 Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Fri, 9 Oct 2026 15:54:12 +1300 Subject: [PATCH 08/28] Require verified, hosted-domain Google identities; RS256 only - ID tokens must have email_verified=true. - When GOOGLE_ALLOWED_DOMAIN is set, the hosted-domain (hd) claim must match it as well as the email domain (case-insensitive). Before, a bare suffix compare on the email was the only check. - The ID token verifier accepts RS256 only. Google never uses HS256, and allowing it only matters if the issuer can be swapped. - GOOGLE_ALLOWED_DOMAIN is required outside DEV_MODE. An empty value used to let any Google account sign in. - The mock OAuth server now emits hd like Google does for Workspace accounts. --- internal/gateway/auth/oauth.go | 48 +++++++-------- internal/gateway/auth/oauth_claims_test.go | 68 ++++++++++++++++++++++ internal/gateway/config/config.go | 4 ++ internal/gateway/config/config_test.go | 11 ++++ mock-oauth/src/signing.ts | 2 + 5 files changed, 109 insertions(+), 24 deletions(-) create mode 100644 internal/gateway/auth/oauth_claims_test.go diff --git a/internal/gateway/auth/oauth.go b/internal/gateway/auth/oauth.go index 008a58f4..dd726e89 100644 --- a/internal/gateway/auth/oauth.go +++ b/internal/gateway/auth/oauth.go @@ -116,7 +116,7 @@ func NewOAuthHandler( // Create ID token verifier with proper configuration verifierConfig := &oidc.Config{ ClientID: clientID, - SupportedSigningAlgs: []string{"RS256", "HS256"}, // Support both for flexibility + SupportedSigningAlgs: []string{oidc.RS256}, } idTokenVerifier := provider.Verifier(verifierConfig) @@ -218,24 +218,12 @@ func (h *OAuthHandler) CompleteLogin(code, _ string, codeVerifier string) (*Logi return nil, fmt.Errorf("failed to verify ID token: %w", err) } - // Extract user claims - var claims struct { - Email string `json:"email"` - EmailVerified bool `json:"email_verified"` - Name string `json:"name"` - HD string `json:"hd,omitempty"` // Google Workspace domain - } - + var claims identityClaims if err := idToken.Claims(&claims); err != nil { return nil, fmt.Errorf("failed to extract claims: %w", err) } - - // Verify email domain if required - if h.allowedDomain != "" && !h.isAllowedDomain(claims.Email) { - return nil, &DomainNotAllowedError{ - Email: claims.Email, - Name: claims.Name, - } + if err := claims.validate(h.allowedDomain); err != nil { + return nil, err } return &LoginResponse{ @@ -244,16 +232,28 @@ func (h *OAuthHandler) CompleteLogin(code, _ string, codeVerifier string) (*Logi }, nil } -// isAllowedDomain checks if email domain is allowed -func (h *OAuthHandler) isAllowedDomain(email string) bool { - if h.allowedDomain == "" { - return true +// identityClaims are the Google ID token claims the gateway relies on. +type identityClaims struct { + Email string `json:"email"` + EmailVerified bool `json:"email_verified"` + Name string `json:"name"` + HD string `json:"hd,omitempty"` // Google Workspace hosted domain; absent for consumer accounts +} + +// validate requires a verified email and, when an allowed domain is configured, that both the email +// domain and the hosted-domain claim match it. +func (c identityClaims) validate(allowedDomain string) error { + if !c.EmailVerified { + return fmt.Errorf("google account email %q is not verified", c.Email) + } + if allowedDomain == "" { + return nil } - parts := strings.Split(email, "@") - if len(parts) != 2 { - return false + parts := strings.Split(c.Email, "@") + if len(parts) != 2 || !strings.EqualFold(parts[1], allowedDomain) || !strings.EqualFold(c.HD, allowedDomain) { + return &DomainNotAllowedError{Email: c.Email, Name: c.Name} } - return parts[1] == h.allowedDomain + return nil } // generateSecureRandomString generates a cryptographically secure random string diff --git a/internal/gateway/auth/oauth_claims_test.go b/internal/gateway/auth/oauth_claims_test.go new file mode 100644 index 00000000..028741e0 --- /dev/null +++ b/internal/gateway/auth/oauth_claims_test.go @@ -0,0 +1,68 @@ +package auth + +import ( + "errors" + "testing" +) + +func TestIdentityClaimsValidate(t *testing.T) { + cases := []struct { + name string + claims identityClaims + allowedDomain string + wantErr bool + wantDomainErr bool + }{ + { + "workspace user", + identityClaims{Email: "a@docspring.com", EmailVerified: true, HD: "docspring.com"}, + "docspring.com", false, false, + }, + { + "domain case differs", + identityClaims{Email: "a@DocSpring.com", EmailVerified: true, HD: "docspring.com"}, + "docspring.com", false, false, + }, + { + "unverified email", + identityClaims{Email: "a@docspring.com", EmailVerified: false, HD: "docspring.com"}, + "docspring.com", true, false, + }, + { + "consumer account with matching email domain", + identityClaims{Email: "a@docspring.com", EmailVerified: true}, + "docspring.com", true, true, + }, + { + "other workspace", + identityClaims{Email: "a@evil.com", EmailVerified: true, HD: "evil.com"}, + "docspring.com", true, true, + }, + { + "hd mismatch", + identityClaims{Email: "a@docspring.com", EmailVerified: true, HD: "evil.com"}, + "docspring.com", true, true, + }, + { + "subdomain", + identityClaims{Email: "a@x.docspring.com", EmailVerified: true, HD: "x.docspring.com"}, + "docspring.com", true, true, + }, + { + "malformed email", + identityClaims{Email: "a@b@docspring.com", EmailVerified: true, HD: "docspring.com"}, + "docspring.com", true, true, + }, + {"no domain configured (dev)", identityClaims{Email: "a@gmail.com", EmailVerified: true}, "", false, false}, + } + for _, tc := range cases { + err := tc.claims.validate(tc.allowedDomain) + if (err != nil) != tc.wantErr { + t.Errorf("%s: err=%v wantErr=%v", tc.name, err, tc.wantErr) + } + var domainErr *DomainNotAllowedError + if errors.As(err, &domainErr) != tc.wantDomainErr { + t.Errorf("%s: DomainNotAllowedError=%v want %v", tc.name, errors.As(err, &domainErr), tc.wantDomainErr) + } + } +} diff --git a/internal/gateway/config/config.go b/internal/gateway/config/config.go index 9cfcef30..0f98cd41 100644 --- a/internal/gateway/config/config.go +++ b/internal/gateway/config/config.go @@ -81,6 +81,10 @@ func Load() (*Config, error) { if err := cfg.loadSessionSecret(); err != nil { return nil, err } + if !cfg.DevMode && strings.TrimSpace(cfg.GoogleAllowedDomain) == "" { + // Without it any Google account could sign in (and be matched to users by email). + return nil, fmt.Errorf("GOOGLE_ALLOWED_DOMAIN is required in production") + } cfg.loadUserRoles() cfg.loadTrustedProxies() diff --git a/internal/gateway/config/config_test.go b/internal/gateway/config/config_test.go index 598d43c8..d158653b 100644 --- a/internal/gateway/config/config_test.go +++ b/internal/gateway/config/config_test.go @@ -62,3 +62,14 @@ func TestLoadProductionRequiresSecret(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "APP_SECRET_KEY is required in production") } + +func TestLoadProductionRequiresAllowedDomain(t *testing.T) { + t.Setenv("DEV_MODE", "false") + t.Setenv("APP_SECRET_KEY", "secret") + t.Setenv("GOOGLE_ALLOWED_DOMAIN", "") + + cfg, err := Load() + assert.Nil(t, cfg) + assert.Error(t, err) + assert.Contains(t, err.Error(), "GOOGLE_ALLOWED_DOMAIN is required in production") +} diff --git a/mock-oauth/src/signing.ts b/mock-oauth/src/signing.ts index 57d7f7f2..c3e4a3cc 100644 --- a/mock-oauth/src/signing.ts +++ b/mock-oauth/src/signing.ts @@ -64,6 +64,8 @@ export const generateMockIdToken = (user: MockUser): Promise => { iat: now, email: user.email, email_verified: user.verified_email, + // Google sets hd (hosted domain) for Google Workspace accounts; every mock user is one. + hd: user.email.split("@")[1], name: user.name, picture: user.picture, }; From 46bf3504ea4069c9f8edc2b71048858a40c331f9 Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Fri, 9 Oct 2026 16:08:42 +1300 Subject: [PATCH 09/28] Fix approval list MFA level and E2E fallout from the MFA session gate - Listing deploy approval requests and reading a request's audit trail were mapped to deploy_approval_request:approve, which carries MFAAlways, so every page load demanded a fresh MFA code. They now use the read-level :list permission (admin-only); the handlers still require approver permission. A test asserts no GET route can require MFAAlways. - A 403 from the Authorize middleware names the missing permission, e.g. "insufficient permissions: requires convox:env:read". - E2E login helper: when login lands on the MFA challenge, complete it with a TOTP code like a user would. Enrolling TOTP directly in the DB for an already logged-in user now also marks that session MFA-verified, matching a real UI enrollment (step-up stays unset). - cli-login-webui clears TOTP replay state before generating its approve code, so another test on the same shard can't have used the time step already. - CLI E2E expectations updated for the new permission error message. --- internal/gateway/middleware/authorize.go | 32 +++++++++---- internal/gateway/rbac/gateway_routes.go | 7 ++- internal/gateway/rbac/http_routes_test.go | 13 ++++++ scripts/lib/cli-e2e/stages.sh | 4 +- web/e2e/account-security.spec.ts | 36 ++++----------- web/e2e/cli-login-webui.spec.ts | 9 +++- web/e2e/db.ts | 18 ++++++++ web/e2e/helpers.ts | 56 ++++++++++++++++++++++- web/src/lib/generated/mfa-requirements.ts | 8 ++-- 9 files changed, 136 insertions(+), 47 deletions(-) diff --git a/internal/gateway/middleware/authorize.go b/internal/gateway/middleware/authorize.go index 516a899d..fd400f50 100644 --- a/internal/gateway/middleware/authorize.go +++ b/internal/gateway/middleware/authorize.go @@ -32,14 +32,14 @@ func Authorize(manager rbac.Manager) gin.HandlerFunc { return } - allowed, err := routeAllowed(manager, spec, authUser.Principal()) + allowed, missing, err := routeAllowed(manager, spec, authUser.Principal()) if err != nil { gtwlog.Errorf("authz: failed to check permissions for %s %s: %v", spec.Method, spec.Pattern, err) c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"error": "failed to check permissions"}) return } if !allowed { - abortForbidden(c, "insufficient permissions") + abortForbidden(c, insufficientPermissionsMessage(missing)) return } @@ -47,25 +47,39 @@ func Authorize(manager rbac.Manager) gin.HandlerFunc { } } -func routeAllowed(manager rbac.Manager, spec rbac.RouteSpec, principal rbac.Principal) (bool, error) { +// routeAllowed reports whether the principal may use the route and, when denied, the first +// permission it lacks (empty when the route has no usable policy). +func routeAllowed(manager rbac.Manager, spec rbac.RouteSpec, principal rbac.Principal) (bool, string, error) { switch spec.Access { case rbac.AccessAuthenticated: - return true, nil + return true, "", nil case rbac.AccessPermissions: permissions := spec.PermissionStrings() if len(permissions) == 0 { - return false, nil + return false, "", nil } for _, permission := range permissions { allowed, err := manager.Authorize(principal, permission) - if err != nil || !allowed { - return false, err + if err != nil { + return false, "", err + } + if !allowed { + return false, permission, nil } } - return true, nil + return true, "", nil default: - return false, nil + return false, "", nil + } +} + +// insufficientPermissionsMessage names the missing permission so CLI and UI users can tell +// what access they need. +func insufficientPermissionsMessage(missing string) string { + if missing == "" { + return "insufficient permissions" } + return "insufficient permissions: requires " + missing } func abortForbidden(c *gin.Context, message string) { diff --git a/internal/gateway/rbac/gateway_routes.go b/internal/gateway/rbac/gateway_routes.go index c0b194c7..5f40d41e 100644 --- a/internal/gateway/rbac/gateway_routes.go +++ b/internal/gateway/rbac/gateway_routes.go @@ -51,12 +51,15 @@ var httpRouteSpecs = []RouteSpec{ {Method: "GET", Pattern: "/api/v1/info", Access: AccessAuthenticated, AllowAPIToken: true}, newSelfRoute("GET", "/api/v1/created-by"), newTokenRoute("GET", "/api/v1/rack", Convox(ResourceRack, ActionRead)), - newHTTPRoute("GET", "/api/v1/deploy-approval-requests", Gateway(ResourceDeployApprovalRequest, ActionApprove)), + // Listing every request (and a request's audit trail) is for approvers. The handlers also check + // approver permission; the route uses the read-level :list permission so these reads don't + // inherit :approve's MFAAlways requirement. + newHTTPRoute("GET", "/api/v1/deploy-approval-requests", Gateway(ResourceDeployApprovalRequest, ActionList)), newTokenRoute("GET", "/api/v1/deploy-approval-requests/:id", Gateway(ResourceDeployApprovalRequest, ActionRead)), newHTTPRoute( "GET", "/api/v1/deploy-approval-requests/:id/audit-logs", - Gateway(ResourceDeployApprovalRequest, ActionApprove), + Gateway(ResourceDeployApprovalRequest, ActionList), ), newTokenRoute("POST", "/api/v1/deploy-approval-requests", Gateway(ResourceDeployApprovalRequest, ActionCreate)), newHTTPRoute( diff --git a/internal/gateway/rbac/http_routes_test.go b/internal/gateway/rbac/http_routes_test.go index 45cb092a..e117b026 100644 --- a/internal/gateway/rbac/http_routes_test.go +++ b/internal/gateway/rbac/http_routes_test.go @@ -124,3 +124,16 @@ func TestWebConvoxMutationRoutesHavePrecisePermissions(t *testing.T) { } }) } + +// TestReadRoutesDoNotRequireInlineMFA ensures plain GET routes never inherit a write permission's +// MFA level. A GET mapped to an MFAAlways permission would demand a fresh MFA code on every page load. +func TestReadRoutesDoNotRequireInlineMFA(t *testing.T) { + for _, spec := range HTTPRouteSpecs() { + if spec.Method != "GET" { + continue + } + if level := spec.GetMFALevel(); level == MFAAlways { + t.Errorf("GET %s requires MFAAlways via %v", spec.Pattern, spec.Permissions) + } + } +} diff --git a/scripts/lib/cli-e2e/stages.sh b/scripts/lib/cli-e2e/stages.sh index 7e173399..6c266da1 100644 --- a/scripts/lib/cli-e2e/stages.sh +++ b/scripts/lib/cli-e2e/stages.sh @@ -412,9 +412,9 @@ run_viewer_tests() { verify_rgw_command "ps" "p-web-1" "p-worker-1" verify_rgw_command_failure "env" \ - "Error: failed to fetch env: You don't have permission to view environment variables" + "Error: failed to fetch env: insufficient permissions: requires convox:env:read" verify_rgw_command_failure "env get DATABASE_URL --unmask" \ - "Error: failed to fetch env: You don't have permission to view environment variables." + "Error: failed to fetch env: insufficient permissions: requires convox:env:read" clear_mfa_replay_protection local delete_code diff --git a/web/e2e/account-security.spec.ts b/web/e2e/account-security.spec.ts index 750fc885..38d5c9fa 100644 --- a/web/e2e/account-security.spec.ts +++ b/web/e2e/account-security.spec.ts @@ -6,7 +6,10 @@ import { expect, test } from './fixtures' import { clearStepUpSessions, clickLoginButton, + completeMfaChallenge, enforceMfaFor, + isOnMfaChallenge, + isOnMfaChallengeUrl, login, resetMfaFor, satisfyMFAStepUpModal, @@ -81,33 +84,10 @@ async function performLoginWithMfa(page: Page, secret: string, trustDevice: bool }) .toBeTruthy() - // Wait to see if we're redirected to MFA challenge page or directly to app - try { - await page.waitForURL(/auth\/mfa\/challenge/, { timeout: 10_000 }) - // We're on the MFA challenge page - fill in the code - const verificationInput = page.getByLabel('Verification code') - await expect(verificationInput).toBeVisible({ timeout: 5000 }) - - if (trustDevice) { - const trustCheckbox = page.getByLabel(/Trust this/i) - const checkboxExists = await trustCheckbox.isVisible().catch(() => false) - if (checkboxExists) { - const currentlyChecked = await trustCheckbox.isChecked().catch(() => false) - if (!currentlyChecked) { - await trustCheckbox.check() - } - } - } - - // Type the code digit by digit to trigger auto-submit - const code = authenticator.generate(secret) - await typeOtpCode(page, page, code) - - // Auto-submits on 6-digit code, wait for redirect - await page.waitForURL(/app(?:\/|$)/, { timeout: 30_000 }) - } catch { - // Not redirected to MFA challenge - might already be at /app or trusted device - await page.waitForURL(/app(?:\/|$)/, { timeout: 30_000 }) + // A trusted device skips the challenge and goes straight to the app. + await page.waitForURL(/app(?:\/|$)/, { timeout: 30_000 }) + if (isOnMfaChallenge(page)) { + await completeMfaChallenge(page, secret, { trustDevice }) } } @@ -392,7 +372,7 @@ test.describe('Account security', () => { const code = authenticator.generate(secret) await typeOtpCode(page, page, code) - await page.waitForURL(/app(?:\/|$)/, { timeout: 15_000 }) + await page.waitForURL((url) => !isOnMfaChallengeUrl(url), { timeout: 15_000 }) }) test('user can set and persist preferred MFA method', async ({ page }) => { diff --git a/web/e2e/cli-login-webui.spec.ts b/web/e2e/cli-login-webui.spec.ts index 859b8c1d..6c5d08f9 100644 --- a/web/e2e/cli-login-webui.spec.ts +++ b/web/e2e/cli-login-webui.spec.ts @@ -15,7 +15,11 @@ */ import { authenticator } from 'otplib' import { APIRoute, WebRoute } from '@/lib/routes' -import { createPendingDeployApprovalRequest, deleteDeployApprovalRequest } from './db' +import { + clearMfaAttempts, + createPendingDeployApprovalRequest, + deleteDeployApprovalRequest, +} from './db' import { expect, test } from './fixtures' import { ensureMfaEnrollment, resetMfaFor } from './helpers' @@ -108,6 +112,9 @@ test.describe('CLI login to WebUI flow', () => { // The approve endpoint requires inline MFA with every request. // The WebUI's MFA dialog handles this by prompting for TOTP and sending X-MFA-TOTP header. // Here we simulate that by generating a TOTP code and sending it with the request. + // Other tests on this shard may have used admin's current TOTP time step; clear replay state + // so this code isn't rejected as a replay. + await clearMfaAttempts() const totpCode = authenticator.generate(secret) const approveResponse = await page.evaluate( diff --git a/web/e2e/db.ts b/web/e2e/db.ts index 62e98bc3..c1e16bcc 100644 --- a/web/e2e/db.ts +++ b/web/e2e/db.ts @@ -467,6 +467,24 @@ export async function setupTotpMfaForUser(email: string) { }) } +/** + * Marks the user's active sessions as having completed their MFA challenge, which is the state a + * real UI enrollment or challenge leaves behind. Use after enrolling MFA directly in the database + * for a user who is already logged in. The step-up timestamp is left alone so step-up prompts + * still appear where a test expects them. + */ +export async function markSessionsMfaVerified(email: string) { + await withDbClient(async (client) => { + await client.query( + `UPDATE user_sessions + SET mfa_verified_at = COALESCE(mfa_verified_at, NOW()) + WHERE user_id = (SELECT id FROM users WHERE email = $1) + AND revoked_at IS NULL;`, + [email] + ) + }) +} + export async function setupBothMfaMethodsForUser(email: string) { await withDbClient(async (client) => { await resetMfaAndSetupTotp(client, email) diff --git a/web/e2e/helpers.ts b/web/e2e/helpers.ts index 159811cf..f4cb804e 100644 --- a/web/e2e/helpers.ts +++ b/web/e2e/helpers.ts @@ -8,6 +8,7 @@ import { expireStepUpForAllSessions, getPendingTotpSecret, getUserMfaSecret, + markSessionsMfaVerified, resetMfaForUser, setupBothMfaMethodsForUser, setupTotpMfaForUser, @@ -102,11 +103,62 @@ export async function login(page: Page, options: LoginOptions = {}) { await page.waitForURL(/app(?:\/|$)/, { timeout: 15_000 }) + // A user who already has MFA lands on the challenge page. Complete it like a real user would, + // otherwise the session stays unverified and every API call is refused. + if (isOnMfaChallenge(page)) { + const secret = await getUserMfaSecret(email) + if (!secret) { + throw new Error(`Login for ${email} reached the MFA challenge but no TOTP secret exists`) + } + await completeMfaChallenge(page, secret) + } + if (autoEnrollMfa) { await ensureMfaEnrollment(page, { email }) } } +export function isOnMfaChallengeUrl(url: URL): boolean { + return url.pathname.includes('/auth/mfa/challenge') +} + +export function isOnMfaChallenge(page: Page): boolean { + return isOnMfaChallengeUrl(new URL(page.url())) +} + +/** + * Completes the MFA challenge page with a TOTP code, the way a user would, and waits until the + * browser has left the challenge page. + */ +export async function completeMfaChallenge( + page: Page, + secret: string, + options: { trustDevice?: boolean } = {} +) { + await clearMfaAttempts() + + const codeInput = page.getByLabel('Verification code') + const useTotpButton = page.getByRole('button', { name: /Use authenticator app instead/i }) + await expect(codeInput.or(useTotpButton).first()).toBeVisible({ timeout: 10_000 }) + if (await useTotpButton.isVisible().catch(() => false)) { + await useTotpButton.click() + } + await expect(codeInput).toBeVisible({ timeout: 5000 }) + + if (options.trustDevice) { + const trustCheckbox = page.getByLabel(/Trust this/i) + if ( + (await trustCheckbox.isVisible().catch(() => false)) && + !(await trustCheckbox.isChecked().catch(() => false)) + ) { + await trustCheckbox.check() + } + } + + await typeOtpCode(page, page, authenticator.generate(secret)) + await page.waitForURL((url) => !isOnMfaChallengeUrl(url), { timeout: 30_000 }) +} + export async function resetMfaFor(email: string) { await resetMfaForUser(email) } @@ -192,8 +244,10 @@ export async function ensureMfaEnrollment( return secret } - // Use DB helper for fast setup instead of UI flow + // Use DB helper for fast setup instead of UI flow. A real enrollment also completes the + // session's MFA challenge, so mark the already-open session as verified. await setupTotpMfaForUser(email) + await markSessionsMfaVerified(email) // Reload page to ensure frontend picks up the new MFA status (e.g. mfa_enrolled flag in user object) // Only reload if we are not already navigating/redirecting significantly diff --git a/web/src/lib/generated/mfa-requirements.ts b/web/src/lib/generated/mfa-requirements.ts index 975b5aee..0587a6cc 100644 --- a/web/src/lib/generated/mfa-requirements.ts +++ b/web/src/lib/generated/mfa-requirements.ts @@ -195,8 +195,8 @@ export const HTTP_ROUTE_MFA_REQUIREMENTS: HttpRouteMfaRequirement[] = [ { method: "GET", pattern: "/api/v1/deploy-approval-requests", - permissions: ["gateway:deploy_approval_request:approve"], - mfaLevel: "always", + permissions: ["gateway:deploy_approval_request:list"], + mfaLevel: "none", }, { method: "GET", @@ -207,8 +207,8 @@ export const HTTP_ROUTE_MFA_REQUIREMENTS: HttpRouteMfaRequirement[] = [ { method: "GET", pattern: "/api/v1/deploy-approval-requests/:id/audit-logs", - permissions: ["gateway:deploy_approval_request:approve"], - mfaLevel: "always", + permissions: ["gateway:deploy_approval_request:list"], + mfaLevel: "none", }, { method: "GET", From 1a5cec2de2d33b244763cbe3b8502a5419f5f8fc Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Fri, 9 Oct 2026 16:20:26 +1300 Subject: [PATCH 10/28] CLI: focus the PIN field in the macOS security key dialog When the CLI runs in the background (e.g. from an agent), osascript isn't the frontmost app, so the dialog appeared without keyboard focus and the PIN field had to be clicked before typing. The script now runs `activate` first, which brings the dialog to the front with the field focused. No extra macOS permissions are needed. --- internal/cli/pinentry/pinentry.go | 4 +++- internal/cli/pinentry/pinentry_test.go | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/internal/cli/pinentry/pinentry.go b/internal/cli/pinentry/pinentry.go index 5dea2a98..6df0dd25 100644 --- a/internal/cli/pinentry/pinentry.go +++ b/internal/cli/pinentry/pinentry.go @@ -31,8 +31,10 @@ const ( ) // dialogScript shows the message passed as the first argument (never interpolated into the script) -// and prints the entered PIN to stdout. +// and prints the entered PIN to stdout. `activate` brings osascript to the front first: when the CLI +// runs in the background (e.g. from an agent) the dialog otherwise opens without keyboard focus. const dialogScript = `on run argv + activate set reply to display dialog (item 1 of argv) with title "rack-gateway" default answer "" ¬ with hidden answer buttons {"Cancel", "Approve"} default button "Approve" cancel button "Cancel" ¬ with icon caution giving up after 170 diff --git a/internal/cli/pinentry/pinentry_test.go b/internal/cli/pinentry/pinentry_test.go index 5a416504..fa867095 100644 --- a/internal/cli/pinentry/pinentry_test.go +++ b/internal/cli/pinentry/pinentry_test.go @@ -97,3 +97,12 @@ func TestDescribeCommandTruncatesLongCommands(t *testing.T) { t.Fatalf("describeCommand length = %d, want %d ending in ...", len(got), maxCommandLength+3) } } + +func TestDialogScriptActivatesBeforeShowingDialog(t *testing.T) { + activate := strings.Index(dialogScript, "\tactivate\n") + dialog := strings.Index(dialogScript, "display dialog") + if activate < 0 || activate > dialog { + t.Fatalf("dialog script must activate osascript before the dialog so the PIN field has focus:\n%s", + dialogScript) + } +} From 31424c58fa9deb5a4cfc49fd48778ae13cfd0a81 Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Fri, 9 Oct 2026 17:00:56 +1300 Subject: [PATCH 11/28] Bind deploy approvals to the approved commit and artifact An approval used to authorize "a build" for the token and app, so a compromised CI job could ship any tarball or image under it: git-sha was optional (DocSpring CI never sends one), the build's archive wasn't compared with the upload, and image patterns were optional and unanchored. Token builds under an approval now: - must use the archive uploaded under that same approval, chosen deterministically (no "most recent approved" lookup); - take the commit from the approval record, never from the client. A git-sha, if sent, must equal it, and approvals need a full 40-hex SHA; - may only reference pre-built images tagged or -. Services built from source are rejected; - require service_image_patterns, so the image repository is pinned too. Patterns are anchored and the substituted commit is regex-escaped. Also: - approved_deploy_commands fails closed: an empty list allows no commands under an approval. The CLI E2E seed stored it as {"commands": [...]}, which was silently read as empty and so allowed anything; it now uses the plain array production uses. - GitHub verification: "branch" mode requires the commit to be on the feature branch (compare status behind/identical, not any 200), "latest" mode compares full SHAs, and URL segments are path-escaped. - CircleCI auto-approve checks the workflow's pipeline revision and repository against the approval (and rejects fork pipelines) before approving the hold job. workflow_id must be a UUID. - CircleCI and GitHub API tokens are no longer stored in River job args; workers read them from config. The Jobs API redacts token-, secret-, password- and key-like arg keys. --- .../integrations/deploy-approvals/index.mdx | 15 +- internal/gateway/app/setup.go | 2 + internal/gateway/circleci/client.go | 19 +- .../gateway/circleci/pipeline_verification.go | 117 +++++++ .../circleci/pipeline_verification_test.go | 109 ++++++ internal/gateway/db/commit_sha.go | 22 ++ internal/gateway/db/database_test.go | 43 ++- .../db/deploy_approval_requests_mutations.go | 6 +- .../db/deploy_approval_requests_queries.go | 18 +- internal/gateway/github/client.go | 128 +++++--- internal/gateway/github/verify_commit_test.go | 94 ++++++ internal/gateway/handlers/admin_jobs.go | 11 +- .../gateway/handlers/deploy_approval_admin.go | 18 +- .../handlers/deploy_approval_notification.go | 9 +- .../deploy_approval_requests_mfa_test.go | 4 +- .../handlers/deploy_approval_requests_test.go | 62 ++-- .../handlers/deploy_approval_validation.go | 7 +- .../gateway/handlers/job_args_redaction.go | 60 ++++ .../handlers/job_args_redaction_test.go | 42 +++ internal/gateway/jobs/circleci/approve.go | 34 +- .../gateway/jobs/circleci/approve_test.go | 23 +- internal/gateway/jobs/client.go | 8 +- internal/gateway/jobs/github/pr_comment.go | 21 +- .../gateway/jobs/github/pr_comment_test.go | 11 +- internal/gateway/proxy/approval_resolvers.go | 309 ++++++++++++++++++ .../proxy/deploy_approval_binding_test.go | 242 ++++++++++++++ .../proxy/deploy_approval_tracking_test.go | 68 ++-- .../proxy/deploy_approval_validation.go | 186 ++++------- internal/gateway/proxy/env_filters.go | 6 +- internal/gateway/proxy/forward.go | 29 +- internal/gateway/proxy/manifest_policy.go | 152 +++++++++ internal/gateway/proxy/manifest_validation.go | 74 +---- internal/gateway/proxy/token_permissions.go | 271 --------------- scripts/lib/cli-e2e/db.sh | 2 +- web/e2e/db.ts | 2 +- 35 files changed, 1556 insertions(+), 668 deletions(-) create mode 100644 internal/gateway/circleci/pipeline_verification.go create mode 100644 internal/gateway/circleci/pipeline_verification_test.go create mode 100644 internal/gateway/db/commit_sha.go create mode 100644 internal/gateway/github/verify_commit_test.go create mode 100644 internal/gateway/handlers/job_args_redaction.go create mode 100644 internal/gateway/handlers/job_args_redaction_test.go create mode 100644 internal/gateway/proxy/approval_resolvers.go create mode 100644 internal/gateway/proxy/deploy_approval_binding_test.go create mode 100644 internal/gateway/proxy/manifest_policy.go diff --git a/docs/src/content/docs/integrations/deploy-approvals/index.mdx b/docs/src/content/docs/integrations/deploy-approvals/index.mdx index 94220c8d..b5f8ac0d 100644 --- a/docs/src/content/docs/integrations/deploy-approvals/index.mdx +++ b/docs/src/content/docs/integrations/deploy-approvals/index.mdx @@ -94,11 +94,14 @@ graph TB ### Git Commit Verification -Every approval is tied to a specific git commit hash: +Every approval is tied to a specific full (40-character) git commit SHA. An API token deploying under an approval must: -- Approval cannot be reused for different code -- Manifest validation ensures deployed images match approved commit -- Prevents deploying arbitrary code even with compromised CI/CD token +- Build the archive it uploaded under that approval (one upload and one build per approval) +- Send no `git-sha`, or one equal to the approved commit (the gateway always uses the approved commit, never the client's) +- Reference only pre-built images, each tagged with the approved commit (`` or `-`, e.g. `-amd64`) and matching the app's `service_image_patterns` +- Run only commands listed in `approved_deploy_commands` (an empty list allows none) + +`service_image_patterns` is required for approval-bound builds: the commit tag alone does not pin the image repository. ### MFA Step-Up @@ -183,7 +186,9 @@ Configure via UI or environment variables: | `vcs_repo` | Repository in org/repo format | | `ci_provider` | CI system (circleci) | | `circleci_approval_job_name` | CircleCI approval job name | -| `circleci_auto_approve_on_approval` | Enable auto-approval | +| `circleci_auto_approve_on_approval` | Enable auto-approval (the gateway first checks the workflow's pipeline builds the approved commit of `vcs_repo`) | +| `service_image_patterns` | **Required.** Map of service name (or `*`) to an anchored regex; `{{GIT_COMMIT}}` is replaced with the approved commit, e.g. `{"*": "docker\\.io/org/app:{{GIT_COMMIT}}-amd64"}` | +| `approved_deploy_commands` | Exact commands a token may run under an approval (e.g. migrations); empty allows none |