Add logging for security events (authentication, access) (#320)

* feat(log): add security events logging for authentication and access control

- Added `Security` method to `log.Logger` with `[SEC]` prefix at `LevelWarn`.
- Added `SecurityStyle` (purple) to `pkg/cli` and `LogSecurity` helper.
- Added security logging for GitHub CLI authentication checks.
- Added security logging for Agentic configuration loading and token validation.
- Added security logging for sandbox escape detection in `local.Medium`.
- Updated MCP service to support logger injection and log tool executions and connections.
- Ensured all security logs include `user` context for better auditability.

* feat(log): add security events logging for authentication and access control

- Added `Security` method to `log.Logger` with `[SEC]` prefix at `LevelWarn`.
- Added `SecurityStyle` (purple) to `pkg/cli` and `LogSecurity` helper.
- Added security logging for GitHub CLI authentication checks.
- Added security logging for Agentic configuration loading and token validation.
- Added security logging for sandbox escape detection in `local.Medium`.
- Updated MCP service to support logger injection and log tool executions and connections.
- Ensured all security logs include `user` context for better auditability.
- Fixed code formatting issues identified by CI.

* feat(log): refine security logging and fix auto-merge CI

- Moved `Security` log level to `LevelError` for better visibility.
- Added robust `log.Username()` helper using `os/user`.
- Differentiated high-risk (Security) and low-risk (Info) MCP tool executions.
- Ensured consistent `user` context in all security-related logs.
- Fixed merge conflict and missing repository context in `auto-merge` CI.
- Fixed comment positioning in `pkg/mcp/mcp.go`.
- Downgraded MCP TCP accept errors to standard `Error` log level.
- Fixed code formatting in `internal/cmd/setup/cmd_github.go`.

* feat(log): finalize security logging and address CI/CodeQL alerts

- Refined `Security` logging: moved to `LevelError` and consistently include `user` context using `os/user`.
- Differentiated MCP tool executions: write/delete are `Security` level, others are `Info`.
- Fixed CodeQL alert: made UniFi TLS verification configurable (defaults to verify).
- Updated UniFi CLI with `--verify-tls` flag and config support.
- Fixed `auto-merge` CI failure by setting `GH_REPO` env var.
- Fixed formatting and unused imports.
- Added tests for UniFi config resolution.

* fix: handle MustServiceFor return values correctly

MustServiceFor returns (T, error), not just T. This was causing build
failures after the rebase.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude <developers@lethean.io>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Snider 2026-02-05 10:26:48 +00:00 committed by GitHub
parent 2f9d55e3fd
commit b819b9432a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 173 additions and 23 deletions

View file

@ -25,6 +25,7 @@ import (
"github.com/host-uk/core/pkg/cli"
"github.com/host-uk/core/pkg/i18n"
coreio "github.com/host-uk/core/pkg/io"
"github.com/host-uk/core/pkg/log"
"github.com/host-uk/core/pkg/repos"
"github.com/spf13/cobra"
)
@ -75,6 +76,7 @@ func runGitHubSetup() error {
// Check gh is authenticated
if !cli.GhAuthenticated() {
cli.LogSecurity("GitHub setup failed: not authenticated", "action", "setup github", "user", log.Username())
return errors.New(i18n.T("cmd.setup.github.error.not_authenticated"))
}

View file

@ -92,9 +92,11 @@ func LoadConfig(dir string) (*Config, error) {
// Validate configuration
if cfg.Token == "" {
log.Security("agentic authentication failed: no token configured", "user", log.Username())
return nil, log.E("agentic.LoadConfig", "no authentication token configured", nil)
}
log.Security("agentic configuration loaded", "user", log.Username(), "baseURL", cfg.BaseURL)
return cfg, nil
}

View file

@ -48,6 +48,7 @@ func NewLogService(opts LogOptions) func(*framework.Core) (any, error) {
logSvc.StyleInfo = func(s string) string { return InfoStyle.Render(s) }
logSvc.StyleWarn = func(s string) string { return WarningStyle.Render(s) }
logSvc.StyleError = func(s string) string { return ErrorStyle.Render(s) }
logSvc.StyleSecurity = func(s string) string { return SecurityStyle.Render(s) }
return &LogService{Service: logSvc}, nil
}
@ -94,3 +95,21 @@ func LogError(msg string, keyvals ...any) {
l.Error(msg, keyvals...)
}
}
// LogSecurity logs a security message if log service is available.
func LogSecurity(msg string, keyvals ...any) {
if l := Log(); l != nil {
// Ensure user context is included if not already present
hasUser := false
for i := 0; i < len(keyvals); i += 2 {
if keyvals[i] == "user" {
hasUser = true
break
}
}
if !hasUser {
keyvals = append(keyvals, "user", log.Username())
}
l.Security(msg, keyvals...)
}
}

View file

@ -48,22 +48,23 @@ const (
// Core styles
var (
SuccessStyle = NewStyle().Bold().Foreground(ColourGreen500)
ErrorStyle = NewStyle().Bold().Foreground(ColourRed500)
WarningStyle = NewStyle().Bold().Foreground(ColourAmber500)
InfoStyle = NewStyle().Foreground(ColourBlue400)
DimStyle = NewStyle().Dim().Foreground(ColourGray500)
MutedStyle = NewStyle().Foreground(ColourGray600)
BoldStyle = NewStyle().Bold()
KeyStyle = NewStyle().Foreground(ColourGray400)
ValueStyle = NewStyle().Foreground(ColourGray200)
AccentStyle = NewStyle().Foreground(ColourCyan500)
LinkStyle = NewStyle().Foreground(ColourBlue500).Underline()
HeaderStyle = NewStyle().Bold().Foreground(ColourGray200)
TitleStyle = NewStyle().Bold().Foreground(ColourBlue500)
CodeStyle = NewStyle().Foreground(ColourGray300)
NumberStyle = NewStyle().Foreground(ColourBlue300)
RepoStyle = NewStyle().Bold().Foreground(ColourBlue500)
SuccessStyle = NewStyle().Bold().Foreground(ColourGreen500)
ErrorStyle = NewStyle().Bold().Foreground(ColourRed500)
WarningStyle = NewStyle().Bold().Foreground(ColourAmber500)
InfoStyle = NewStyle().Foreground(ColourBlue400)
SecurityStyle = NewStyle().Bold().Foreground(ColourPurple500)
DimStyle = NewStyle().Dim().Foreground(ColourGray500)
MutedStyle = NewStyle().Foreground(ColourGray600)
BoldStyle = NewStyle().Bold()
KeyStyle = NewStyle().Foreground(ColourGray400)
ValueStyle = NewStyle().Foreground(ColourGray200)
AccentStyle = NewStyle().Foreground(ColourCyan500)
LinkStyle = NewStyle().Foreground(ColourBlue500).Underline()
HeaderStyle = NewStyle().Bold().Foreground(ColourGray200)
TitleStyle = NewStyle().Bold().Foreground(ColourBlue500)
CodeStyle = NewStyle().Foreground(ColourGray300)
NumberStyle = NewStyle().Foreground(ColourBlue300)
RepoStyle = NewStyle().Bold().Foreground(ColourBlue500)
)
// Truncate shortens a string to max length with ellipsis.

View file

@ -10,6 +10,7 @@ import (
"time"
"github.com/host-uk/core/pkg/i18n"
"github.com/host-uk/core/pkg/log"
)
// GhAuthenticated checks if the GitHub CLI is authenticated.
@ -17,7 +18,15 @@ import (
func GhAuthenticated() bool {
cmd := exec.Command("gh", "auth", "status")
output, _ := cmd.CombinedOutput()
return strings.Contains(string(output), "Logged in")
authenticated := strings.Contains(string(output), "Logged in")
if authenticated {
LogSecurity("GitHub CLI authenticated", "user", log.Username())
} else {
LogSecurity("GitHub CLI not authenticated", "user", log.Username())
}
return authenticated
}
// ConfirmOption configures Confirm behaviour.

View file

@ -7,6 +7,8 @@ import (
"os"
"path/filepath"
"strings"
"github.com/host-uk/core/pkg/log"
)
// Medium is a local filesystem storage backend.
@ -83,6 +85,7 @@ func (m *Medium) validatePath(p string) (string, error) {
// Verify the resolved part is still within the root
rel, err := filepath.Rel(m.root, realNext)
if err != nil || strings.HasPrefix(rel, "..") {
log.Security("sandbox escape detected", "root", m.root, "path", p, "attempted", realNext, "user", log.Username())
return "", os.ErrPermission // Path escapes sandbox
}
current = realNext

View file

@ -17,6 +17,7 @@ import (
"fmt"
"io"
"os"
"os/user"
"sync"
"time"
)
@ -68,6 +69,7 @@ type Logger struct {
StyleInfo func(string) string
StyleWarn func(string) string
StyleError func(string) string
StyleSecurity func(string) string
}
// RotationOptions defines the log rotation and retention policy.
@ -121,6 +123,7 @@ func New(opts Options) *Logger {
StyleInfo: identity,
StyleWarn: identity,
StyleError: identity,
StyleSecurity: identity,
}
}
@ -244,6 +247,28 @@ func (l *Logger) Error(msg string, keyvals ...any) {
}
}
// Security logs a security event with optional key-value pairs.
// It uses LevelError to ensure security events are visible even in restrictive
// log configurations.
func (l *Logger) Security(msg string, keyvals ...any) {
if l.shouldLog(LevelError) {
l.log(LevelError, l.StyleSecurity("[SEC]"), msg, keyvals...)
}
}
// Username returns the current system username.
// It uses os/user for reliability and falls back to environment variables.
func Username() string {
if u, err := user.Current(); err == nil {
return u.Username
}
// Fallback for environments where user lookup might fail
if u := os.Getenv("USER"); u != "" {
return u
}
return os.Getenv("USERNAME")
}
// --- Default logger ---
var defaultLogger = New(Options{Level: LevelInfo})
@ -282,3 +307,8 @@ func Warn(msg string, keyvals ...any) {
func Error(msg string, keyvals ...any) {
defaultLogger.Error(msg, keyvals...)
}
// Security logs to the default logger.
func Security(msg string, keyvals ...any) {
defaultLogger.Security(msg, keyvals...)
}

View file

@ -39,6 +39,9 @@ func TestLogger_Levels(t *testing.T) {
{"info at quiet", LevelQuiet, (*Logger).Info, false},
{"warn at quiet", LevelQuiet, (*Logger).Warn, false},
{"error at quiet", LevelQuiet, (*Logger).Error, false},
{"security at info", LevelInfo, (*Logger).Security, true},
{"security at error", LevelError, (*Logger).Security, true},
}
for _, tt := range tests {
@ -126,6 +129,24 @@ func TestLevel_String(t *testing.T) {
}
}
func TestLogger_Security(t *testing.T) {
var buf bytes.Buffer
l := New(Options{Level: LevelError, Output: &buf})
l.Security("unauthorized access", "user", "admin")
output := buf.String()
if !strings.Contains(output, "[SEC]") {
t.Error("expected [SEC] prefix in security log")
}
if !strings.Contains(output, "unauthorized access") {
t.Error("expected message in security log")
}
if !strings.Contains(output, "user=admin") {
t.Error("expected context in security log")
}
}
func TestDefault(t *testing.T) {
// Default logger should exist
if Default() == nil {

View file

@ -19,13 +19,22 @@ import (
// For full GUI features, use the core-gui package.
type Service struct {
server *mcp.Server
workspaceRoot string // Root directory for file operations (empty = unrestricted)
medium io.Medium // Filesystem medium for sandboxed operations
workspaceRoot string // Root directory for file operations (empty = unrestricted)
medium io.Medium // Filesystem medium for sandboxed operations
logger *log.Logger // Logger for security events
}
// Option configures a Service.
type Option func(*Service) error
// WithLogger sets the logger for the MCP service.
func WithLogger(l *log.Logger) Option {
return func(s *Service) error {
s.logger = l
return nil
}
}
// WithWorkspaceRoot restricts file operations to the given directory.
// All paths are validated to be within this directory.
// An empty string disables the restriction (not recommended).
@ -63,7 +72,10 @@ func New(opts ...Option) (*Service, error) {
}
server := mcp.NewServer(impl, nil)
s := &Service{server: server}
s := &Service{
server: server,
logger: log.Default(),
}
// Default to current working directory with sandboxed medium
cwd, err := os.Getwd()
@ -280,6 +292,7 @@ type EditDiffOutput struct {
// Tool handlers
func (s *Service) readFile(ctx context.Context, req *mcp.CallToolRequest, input ReadFileInput) (*mcp.CallToolResult, ReadFileOutput, error) {
s.logger.Info("MCP tool execution", "tool", "file_read", "path", input.Path, "user", log.Username())
content, err := s.medium.Read(input.Path)
if err != nil {
log.Error("mcp: read file failed", "path", input.Path, "err", err)
@ -293,6 +306,7 @@ func (s *Service) readFile(ctx context.Context, req *mcp.CallToolRequest, input
}
func (s *Service) writeFile(ctx context.Context, req *mcp.CallToolRequest, input WriteFileInput) (*mcp.CallToolResult, WriteFileOutput, error) {
s.logger.Security("MCP tool execution", "tool", "file_write", "path", input.Path, "user", log.Username())
// Medium.Write creates parent directories automatically
if err := s.medium.Write(input.Path, input.Content); err != nil {
log.Error("mcp: write file failed", "path", input.Path, "err", err)
@ -302,6 +316,7 @@ func (s *Service) writeFile(ctx context.Context, req *mcp.CallToolRequest, input
}
func (s *Service) listDirectory(ctx context.Context, req *mcp.CallToolRequest, input ListDirectoryInput) (*mcp.CallToolResult, ListDirectoryOutput, error) {
s.logger.Info("MCP tool execution", "tool", "dir_list", "path", input.Path, "user", log.Username())
entries, err := s.medium.List(input.Path)
if err != nil {
log.Error("mcp: list directory failed", "path", input.Path, "err", err)
@ -325,6 +340,7 @@ func (s *Service) listDirectory(ctx context.Context, req *mcp.CallToolRequest, i
}
func (s *Service) createDirectory(ctx context.Context, req *mcp.CallToolRequest, input CreateDirectoryInput) (*mcp.CallToolResult, CreateDirectoryOutput, error) {
s.logger.Security("MCP tool execution", "tool", "dir_create", "path", input.Path, "user", log.Username())
if err := s.medium.EnsureDir(input.Path); err != nil {
log.Error("mcp: create directory failed", "path", input.Path, "err", err)
return nil, CreateDirectoryOutput{}, fmt.Errorf("failed to create directory: %w", err)
@ -333,6 +349,7 @@ func (s *Service) createDirectory(ctx context.Context, req *mcp.CallToolRequest,
}
func (s *Service) deleteFile(ctx context.Context, req *mcp.CallToolRequest, input DeleteFileInput) (*mcp.CallToolResult, DeleteFileOutput, error) {
s.logger.Security("MCP tool execution", "tool", "file_delete", "path", input.Path, "user", log.Username())
if err := s.medium.Delete(input.Path); err != nil {
log.Error("mcp: delete file failed", "path", input.Path, "err", err)
return nil, DeleteFileOutput{}, fmt.Errorf("failed to delete file: %w", err)
@ -341,6 +358,7 @@ func (s *Service) deleteFile(ctx context.Context, req *mcp.CallToolRequest, inpu
}
func (s *Service) renameFile(ctx context.Context, req *mcp.CallToolRequest, input RenameFileInput) (*mcp.CallToolResult, RenameFileOutput, error) {
s.logger.Security("MCP tool execution", "tool", "file_rename", "oldPath", input.OldPath, "newPath", input.NewPath, "user", log.Username())
if err := s.medium.Rename(input.OldPath, input.NewPath); err != nil {
log.Error("mcp: rename file failed", "oldPath", input.OldPath, "newPath", input.NewPath, "err", err)
return nil, RenameFileOutput{}, fmt.Errorf("failed to rename file: %w", err)
@ -349,6 +367,7 @@ func (s *Service) renameFile(ctx context.Context, req *mcp.CallToolRequest, inpu
}
func (s *Service) fileExists(ctx context.Context, req *mcp.CallToolRequest, input FileExistsInput) (*mcp.CallToolResult, FileExistsOutput, error) {
s.logger.Info("MCP tool execution", "tool", "file_exists", "path", input.Path, "user", log.Username())
info, err := s.medium.Stat(input.Path)
if err != nil {
// Any error from Stat (e.g., not found, permission denied) is treated as "does not exist"
@ -364,11 +383,13 @@ func (s *Service) fileExists(ctx context.Context, req *mcp.CallToolRequest, inpu
}
func (s *Service) detectLanguage(ctx context.Context, req *mcp.CallToolRequest, input DetectLanguageInput) (*mcp.CallToolResult, DetectLanguageOutput, error) {
s.logger.Info("MCP tool execution", "tool", "lang_detect", "path", input.Path, "user", log.Username())
lang := detectLanguageFromPath(input.Path)
return nil, DetectLanguageOutput{Language: lang, Path: input.Path}, nil
}
func (s *Service) getSupportedLanguages(ctx context.Context, req *mcp.CallToolRequest, input GetSupportedLanguagesInput) (*mcp.CallToolResult, GetSupportedLanguagesOutput, error) {
s.logger.Info("MCP tool execution", "tool", "lang_list", "user", log.Username())
languages := []LanguageInfo{
{ID: "typescript", Name: "TypeScript", Extensions: []string{".ts", ".tsx"}},
{ID: "javascript", Name: "JavaScript", Extensions: []string{".js", ".jsx"}},
@ -390,6 +411,7 @@ func (s *Service) getSupportedLanguages(ctx context.Context, req *mcp.CallToolRe
}
func (s *Service) editDiff(ctx context.Context, req *mcp.CallToolRequest, input EditDiffInput) (*mcp.CallToolResult, EditDiffOutput, error) {
s.logger.Security("MCP tool execution", "tool", "file_edit", "path", input.Path, "user", log.Username())
if input.OldString == "" {
return nil, EditDiffOutput{}, fmt.Errorf("old_string cannot be empty")
}

View file

@ -48,7 +48,7 @@ func (s *Service) ServeTCP(ctx context.Context, addr string) error {
if addr == "" {
addr = t.listener.Addr().String()
}
log.Info("MCP TCP server listening", "addr", addr)
s.logger.Security("MCP TCP server listening", "addr", addr, "user", log.Username())
for {
conn, err := t.listener.Accept()
@ -57,11 +57,12 @@ func (s *Service) ServeTCP(ctx context.Context, addr string) error {
case <-ctx.Done():
return nil
default:
log.Error("mcp: accept error", "err", err)
s.logger.Error("MCP TCP accept error", "err", err, "user", log.Username())
continue
}
}
s.logger.Security("MCP TCP connection accepted", "remote", conn.RemoteAddr().String(), "user", log.Username())
go s.handleConnection(ctx, conn)
}
}
@ -83,7 +84,7 @@ func (s *Service) handleConnection(ctx context.Context, conn net.Conn) {
// Run server (blocks until connection closed)
// Server.Run calls Connect, then Read loop.
if err := server.Run(ctx, transport); err != nil {
log.Error("mcp: connection error", "err", err, "remote", conn.RemoteAddr())
s.logger.Error("MCP TCP connection error", "err", err, "remote", conn.RemoteAddr().String(), "user", log.Username())
}
}

40
pkg/unifi/config_test.go Normal file
View file

@ -0,0 +1,40 @@
package unifi
import (
"os"
"testing"
"github.com/stretchr/testify/assert"
)
func TestResolveConfig(t *testing.T) {
// Set env vars
os.Setenv("UNIFI_URL", "https://env-url")
os.Setenv("UNIFI_USER", "env-user")
os.Setenv("UNIFI_PASS", "env-pass")
os.Setenv("UNIFI_VERIFY_TLS", "false")
defer func() {
os.Unsetenv("UNIFI_URL")
os.Unsetenv("UNIFI_USER")
os.Unsetenv("UNIFI_PASS")
os.Unsetenv("UNIFI_VERIFY_TLS")
}()
url, user, pass, apikey, verifyTLS, err := ResolveConfig("", "", "", "", nil)
assert.NoError(t, err)
assert.Equal(t, "https://env-url", url)
assert.Equal(t, "env-user", user)
assert.Equal(t, "env-pass", pass)
assert.Equal(t, "", apikey)
assert.False(t, verifyTLS)
// Flag overrides
url, user, pass, apikey, verifyTLS, err = ResolveConfig("https://flag-url", "flag-user", "flag-pass", "flag-apikey", nil)
assert.NoError(t, err)
assert.Equal(t, "https://flag-url", url)
assert.Equal(t, "flag-user", user)
assert.Equal(t, "flag-pass", pass)
assert.Equal(t, "flag-apikey", apikey)
// Env var for verifyTLS still applies if not overridden in ResolveConfig (which it isn't currently via flags)
assert.False(t, verifyTLS)
}