Skip to content

sec(api): gate GET /api/config and testAccountCredentials on correct permission verbs #1068

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cristim merged 1 commit into from
Jun 7, 2026
Merged

sec(api): gate GET /api/config and testAccountCredentials on correct permission verbs #1068

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion internal/api/handler_accounts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,11 +751,14 @@ func awsAmbientCredResult(acct *config.CloudAccount) (AccountTestResult, bool) {
}

// testAccountCredentials handles POST /api/accounts/:id/test.
// This operation performs live outbound cloud API calls / credential probing,
// which is a write-class side effect. Gate on update:accounts (not view:accounts)
// so the handler is self-protecting if the route Auth level is ever relaxed (02-M5).
func (h *Handler) testAccountCredentials(ctx context.Context, req *events.LambdaFunctionURLRequest, id string) (any, error) {
if err := validateUUID(id); err != nil {
return nil, err
}
session, err := h.requirePermission(ctx, req, "view", "accounts")
session, err := h.requirePermission(ctx, req, "update", "accounts")
if err != nil {
return nil, err
}
Expand Down
29 changes: 29 additions & 0 deletions internal/api/handler_accounts_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1915,3 +1915,32 @@ func (m *mockConfigStoreAccounts) SavePurchaseExecutionTx(ctx context.Context, _
func (m *mockConfigStoreAccounts) WithTx(_ context.Context, fn func(tx pgx.Tx) error) error {
return fn(nil)
}

// Regression test for 02-M5: testAccountCredentials must gate on update:accounts
// (not view:accounts) to match its write-class side effect. A caller holding only
// view:accounts must be rejected with 403.
func TestTestAccountCredentials_RequiresUpdateAccounts(t *testing.T) {
ctx := context.Background()
mockAuth := new(MockAuthService)

viewOnlySession := &Session{
UserID: "dddddddd-dddd-dddd-dddd-dddddddddddd",
Email: "viewonly@example.com",
}

// The caller has a valid session and view:accounts, but NOT update:accounts.
mockAuth.On("ValidateSession", ctx, "view-token").Return(viewOnlySession, nil)
mockAuth.On("HasPermissionAPI", ctx, viewOnlySession.UserID, "update", "accounts").Return(false, nil)

handler := &Handler{auth: mockAuth, config: setupAdminMock(ctx)}

req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{"Authorization": "Bearer view-token"},
}
result, err := handler.testAccountCredentials(ctx, req, "11111111-1111-1111-1111-111111111111")
require.Error(t, err, "view-only caller must be rejected")
assert.Nil(t, result)
ce, ok := IsClientError(err)
require.True(t, ok, "expected ClientError, got %T: %v", err, err)
assert.Equal(t, 403, ce.code, "expected 403, got %d", ce.code)
}
14 changes: 13 additions & 1 deletion internal/api/handler_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ func sourceCloud() string {

// Configuration handlers
func (h *Handler) getConfig(ctx context.Context, req *events.LambdaFunctionURLRequest) (*ConfigResponse, error) {
// Require view:config permission. Every other read handler in the package
// pairs the route-level AuthUser gate with this explicit permission check;
// config GET must be consistent (02-M4).
if _, err := h.requirePermission(ctx, req, "view", "config"); err != nil {
return nil, err
}

globalCfg, err := h.config.GetGlobalConfig(ctx)
if err != nil {
return nil, err
Expand Down Expand Up @@ -179,7 +186,12 @@ func mergeServiceConfig(ctx context.Context, store config.StoreInterface, cfg co
return cfg, nil
}

func (h *Handler) getServiceConfig(ctx context.Context, service string) (any, error) {
func (h *Handler) getServiceConfig(ctx context.Context, req *events.LambdaFunctionURLRequest, service string) (any, error) {
// Require view:config permission. Consistent with getConfig (02-M4).
if _, err := h.requirePermission(ctx, req, "view", "config"); err != nil {
return nil, err
}

// Validate for path traversal attacks
if err := validateServicePath(service); err != nil {
return nil, err
Expand Down
147 changes: 132 additions & 15 deletions internal/api/handler_config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,12 @@ func TestHandler_updateConfig_InvalidBody(t *testing.T) {
func TestHandler_getServiceConfig(t *testing.T) {
ctx := context.Background()
mockStore := new(MockConfigStore)
mockAuth := new(MockAuthService)

adminSession := &Session{
UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
Email: "admin@example.com",
}

serviceCfg := &config.ServiceConfig{
Provider: "aws",
Expand All @@ -126,11 +132,16 @@ func TestHandler_getServiceConfig(t *testing.T) {
Coverage: 80,
}

mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
mockAuth.grantAdmin()
mockStore.On("GetServiceConfig", ctx, "aws", "rds").Return(serviceCfg, nil)

handler := &Handler{config: mockStore}
handler := &Handler{config: mockStore, auth: mockAuth}

result, err := handler.getServiceConfig(ctx, "aws/rds")
req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{"Authorization": "Bearer admin-token"},
}
result, err := handler.getServiceConfig(ctx, req, "aws/rds")
require.NoError(t, err)

cfg := result.(*config.ServiceConfig)
Expand All @@ -140,9 +151,17 @@ func TestHandler_getServiceConfig(t *testing.T) {

func TestHandler_getServiceConfig_InvalidFormat(t *testing.T) {
ctx := context.Background()
handler := &Handler{corsAllowedOrigin: "*"}
mockAuth := new(MockAuthService)
adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"}
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
mockAuth.grantAdmin()

handler := &Handler{corsAllowedOrigin: "*", auth: mockAuth}

result, err := handler.getServiceConfig(ctx, "invalid-format")
req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{"Authorization": "Bearer admin-token"},
}
result, err := handler.getServiceConfig(ctx, req, "invalid-format")
assert.Error(t, err)
assert.Nil(t, result)
assert.Contains(t, err.Error(), "invalid service path")
Expand All @@ -151,12 +170,19 @@ func TestHandler_getServiceConfig_InvalidFormat(t *testing.T) {
func TestHandler_getServiceConfig_NotFound(t *testing.T) {
ctx := context.Background()
mockStore := new(MockConfigStore)
mockAuth := new(MockAuthService)

adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"}
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
mockAuth.grantAdmin()
mockStore.On("GetServiceConfig", ctx, "aws", "unknown").Return(nil, nil)

handler := &Handler{config: mockStore}
handler := &Handler{config: mockStore, auth: mockAuth}

result, err := handler.getServiceConfig(ctx, "aws/unknown")
req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{"Authorization": "Bearer admin-token"},
}
result, err := handler.getServiceConfig(ctx, req, "aws/unknown")
require.NoError(t, err)

// Returns empty response for not found
Expand Down Expand Up @@ -333,12 +359,18 @@ func TestHandler_updateServiceConfig_NoSlash(t *testing.T) {
func TestHandler_getConfig_GlobalConfigError(t *testing.T) {
ctx := context.Background()
mockStore := new(MockConfigStore)
mockAuth := new(MockAuthService)

adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"}
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
mockAuth.grantAdmin()
mockStore.On("GetGlobalConfig", ctx).Return(nil, assert.AnError)

handler := &Handler{config: mockStore}
handler := &Handler{config: mockStore, auth: mockAuth}

req := &events.LambdaFunctionURLRequest{}
req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{"Authorization": "Bearer admin-token"},
}
result, err := handler.getConfig(ctx, req)
assert.Error(t, err)
assert.Nil(t, result)
Expand All @@ -347,6 +379,11 @@ func TestHandler_getConfig_GlobalConfigError(t *testing.T) {
func TestHandler_getConfig_ListServiceConfigsError(t *testing.T) {
ctx := context.Background()
mockStore := new(MockConfigStore)
mockAuth := new(MockAuthService)

adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"}
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
mockAuth.grantAdmin()

globalCfg := &config.GlobalConfig{
EnabledProviders: []string{"aws"},
Expand All @@ -355,9 +392,11 @@ func TestHandler_getConfig_ListServiceConfigsError(t *testing.T) {
mockStore.On("GetGlobalConfig", ctx).Return(globalCfg, nil)
mockStore.On("ListServiceConfigs", ctx).Return(nil, assert.AnError)

handler := &Handler{config: mockStore}
handler := &Handler{config: mockStore, auth: mockAuth}

req := &events.LambdaFunctionURLRequest{}
req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{"Authorization": "Bearer admin-token"},
}
result, err := handler.getConfig(ctx, req)
assert.Error(t, err)
assert.Nil(t, result)
Expand Down Expand Up @@ -396,7 +435,12 @@ func TestHandler_getConfig_SourceIdentity_AdminOnly(t *testing.T) {
})

t.Run("regression #407: non-admin does not receive SourceIdentity", func(t *testing.T) {
mockAuth.On("HasPermissionAPI", ctx, "regular-user", mock.Anything, mock.Anything).Return(false, nil)
// The regular user has view:config permission (passes requirePermission)
// but does not hold admin:* (requireAdmin fails), so SourceIdentity is
// withheld. We match the specific permission verbs so the admin:* call
// (action="admin", resource="*") still returns false.
mockAuth.On("HasPermissionAPI", ctx, "regular-user", "view", "config").Return(true, nil)
mockAuth.On("HasPermissionAPI", ctx, "regular-user", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return(false, nil).Maybe()
req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{"Authorization": "Bearer user-token"},
}
Expand All @@ -410,12 +454,19 @@ func TestHandler_getConfig_SourceIdentity_AdminOnly(t *testing.T) {
func TestHandler_getServiceConfig_Error(t *testing.T) {
ctx := context.Background()
mockStore := new(MockConfigStore)
mockAuth := new(MockAuthService)

adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"}
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
mockAuth.grantAdmin()
mockStore.On("GetServiceConfig", ctx, "aws", "rds").Return(nil, assert.AnError)

handler := &Handler{config: mockStore}
handler := &Handler{config: mockStore, auth: mockAuth}

result, err := handler.getServiceConfig(ctx, "aws/rds")
req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{"Authorization": "Bearer admin-token"},
}
result, err := handler.getServiceConfig(ctx, req, "aws/rds")
assert.Error(t, err)
assert.Nil(t, result)
}
Expand Down Expand Up @@ -521,9 +572,17 @@ func TestHandler_updateServiceConfig_SaveError(t *testing.T) {

func TestHandler_getServiceConfig_InvalidProvider(t *testing.T) {
ctx := context.Background()
handler := &Handler{}
mockAuth := new(MockAuthService)
adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"}
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
mockAuth.grantAdmin()

handler := &Handler{auth: mockAuth}

result, err := handler.getServiceConfig(ctx, "invalid/rds")
req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{"Authorization": "Bearer admin-token"},
}
result, err := handler.getServiceConfig(ctx, req, "invalid/rds")
assert.Error(t, err)
assert.Nil(t, result)
assert.Contains(t, err.Error(), "invalid provider")
Expand Down Expand Up @@ -651,3 +710,61 @@ func TestHandler_updateConfig_PropagationListError(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "updated", result.Status)
}

// Regression tests for 02-M4: GET /api/config and GET /api/config/service/*
// must enforce requirePermission("view","config") and return 403 to callers
// who lack that permission, even though the route is only AuthUser-gated.

func TestHandler_getConfig_ViewConfigPermission_Enforced(t *testing.T) {
ctx := context.Background()
mockStore := new(MockConfigStore)
mockAuth := new(MockAuthService)

userSession := &Session{
UserID: "cccccccc-cccc-cccc-cccc-cccccccccccc",
Email: "viewer@example.com",
}
// User has a valid session but view:config is revoked.
mockAuth.On("ValidateSession", ctx, "user-token").Return(userSession, nil)
mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "view", "config").Return(false, nil)

handler := &Handler{config: mockStore, auth: mockAuth}

req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{"Authorization": "Bearer user-token"},
}
result, err := handler.getConfig(ctx, req)
require.Error(t, err, "must be rejected when view:config is revoked")
assert.Nil(t, result)
ce, ok := IsClientError(err)
require.True(t, ok, "expected ClientError, got %T: %v", err, err)
assert.Equal(t, 403, ce.code, "expected 403, got %d", ce.code)
mockStore.AssertNotCalled(t, "GetGlobalConfig", mock.Anything)
}

func TestHandler_getServiceConfig_ViewConfigPermission_Enforced(t *testing.T) {
ctx := context.Background()
mockStore := new(MockConfigStore)
mockAuth := new(MockAuthService)

userSession := &Session{
UserID: "cccccccc-cccc-cccc-cccc-cccccccccccc",
Email: "viewer@example.com",
}
// User has a valid session but view:config is revoked.
mockAuth.On("ValidateSession", ctx, "user-token").Return(userSession, nil)
mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "view", "config").Return(false, nil)

handler := &Handler{config: mockStore, auth: mockAuth}

req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{"Authorization": "Bearer user-token"},
}
result, err := handler.getServiceConfig(ctx, req, "aws/rds")
require.Error(t, err, "must be rejected when view:config is revoked")
assert.Nil(t, result)
ce, ok := IsClientError(err)
require.True(t, ok, "expected ClientError, got %T: %v", err, err)
assert.Equal(t, 403, ce.code, "expected 403, got %d", ce.code)
mockStore.AssertNotCalled(t, "GetServiceConfig", mock.Anything, mock.Anything, mock.Anything)
}
2 changes: 1 addition & 1 deletion internal/api/router.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ func (r *Router) updateConfigHandler(ctx context.Context, req *events.LambdaFunc
}

func (r *Router) getServiceConfigHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) {
return r.h.getServiceConfig(ctx, params["id"])
return r.h.getServiceConfig(ctx, req, params["id"])
}

func (r *Router) updateServiceConfigHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) {
Expand Down
Loading