Skip to content

Add Audit Logging for Security Events #304

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

Open
iqbalcodes6602 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add Audit Logging for Security Events #304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
25321a3
adding the audit logger files and congif
iqbalcodes6602 Feb 5, 2026
5632549
add audit logger to security events
iqbalcodes6602 Feb 5, 2026
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
102 changes: 102 additions & 0 deletions ground-control/internal/logger/audit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package logger

import (
"context"
"net"
"net/http"
"os"
"sync"
"time"

"github.com/google/uuid"
"github.com/rs/zerolog"
)

type AuditEvent struct {
EventID string `json:"event_id"`
Timestamp time.Time `json:"timestamp"`
EventType string `json:"event_type"`
Actor string `json:"actor,omitempty"`
SourceIP string `json:"source_ip,omitempty"`
Details map[string]interface{} `json:"details,omitempty"`
}

var (
auditLogger *zerolog.Logger
auditLoggerOnce sync.Once
)
Comment on lines +24 to +27
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Replace package-level audit logger globals to satisfy lint rules.

Package-level state violates the “no globals” lint rule and will fail strict golangci-lint. Please refactor to an instance-based logger (e.g., attach an AuditLogger to Server or pass it explicitly).

As per coding guidelines: no global variables (gochecknoglobals).

🤖 Prompt for AI Agents 
In `@ground-control/internal/logger/audit.go` around lines 24 - 27, The
package-level globals auditLogger and auditLoggerOnce must be removed and
replaced with an instance-based logger: create an AuditLogger type (or add a
field to the existing Server struct) that holds a zerolog.Logger and any
initialization state, provide a constructor like NewAuditLogger(ctx, cfg...) to
initialize and return the instance, update all call sites that referenced the
globals to use the instance (e.g., server.AuditLogger or pass *AuditLogger into
functions), and eliminate sync.Once usage by ensuring the constructor handles
safe initialization; reference the symbols auditLogger and auditLoggerOnce to
locate and replace the globals with the new instance-based pattern.


// getAuditLogger lazily initializes the audit logger.
// Configuration is controlled via environment variable AUDIT_LOG_PATH.
func getAuditLogger() *zerolog.Logger {
auditLoggerOnce.Do(func() {
path := os.Getenv("AUDIT_LOG_PATH")
var w *os.File
if path == "" {
w = os.Stdout
} else {
f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600)
if err != nil {
return
}
w = f
}

l := zerolog.New(w).With().Timestamp().Logger()
auditLogger = &l
})

return auditLogger
Comment on lines +31 to +49
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Silent init failure permanently disables audit logging.

If the file open fails, the logger stays nil and sync.Once prevents any retry or fallback. Consider logging the error and allowing a retry or a fallback to stdout to avoid silent loss of audit events.

🤖 Prompt for AI Agents 
In `@ground-control/internal/logger/audit.go` around lines 31 - 49, The
getAuditLogger initialization currently returns early on os.OpenFile failure
which leaves auditLogger nil and auditLoggerOnce preventing retries; update
getAuditLogger so that when OpenFile(path, ...) returns an error it logs that
error (including the error value) and falls back to using os.Stdout for w
instead of returning, then continue to create and assign auditLogger (so
auditLogger is never left nil); reference the getAuditLogger function,
auditLoggerOnce, AUDIT_LOG_PATH env var and the auditLogger variable when making
this change.

}

func LogEvent(_ context.Context, eventType, actor, sourceIP string, details map[string]interface{}) {
l := getAuditLogger()
if l == nil {
return
}

event := AuditEvent{
EventID: uuid.NewString(),
Timestamp: time.Now().UTC(),
EventType: eventType,
Actor: actor,
SourceIP: sourceIP,
Details: details,
}

e := l.Info().
Str("event_id", event.EventID).
Time("timestamp", event.Timestamp).
Str("event_type", event.EventType)

if event.Actor != "" {
e = e.Str("actor", event.Actor)
}
if event.SourceIP != "" {
e = e.Str("source_ip", event.SourceIP)
}
if len(event.Details) > 0 {
e = e.Fields(event.Details)
}

e.Msg("")
}

// ClientIP extracts the best-effort client IP for audit logging.
func ClientIP(r *http.Request) string {
if r == nil {
return ""
}

// Prefer X-Forwarded-For if present
if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
return xff
}

host, _, err := net.SplitHostPort(r.RemoteAddr)
if err != nil {
return r.RemoteAddr
}
return host
}

13 changes: 13 additions & 0 deletions ground-control/internal/server/auth_handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/container-registry/harbor-satellite/ground-control/internal/auth"
"github.com/container-registry/harbor-satellite/ground-control/internal/database"
auditLogger "github.com/container-registry/harbor-satellite/ground-control/internal/logger"
)

const maxFailedAttempts = 5
Expand All @@ -32,6 +33,9 @@ func (s *Server) loginHandler(w http.ResponseWriter, r *http.Request) {
}

if req.Username == "" || req.Password == "" {
auditLogger.LogEvent(r.Context(), "user.auth.failure", req.Username, auditLogger.ClientIP(r), map[string]interface{}{
"reason": "missing_credentials",
})
WriteJSONError(w, "Invalid credentials", http.StatusUnauthorized)
return
}
Expand All @@ -44,6 +48,9 @@ func (s *Server) loginHandler(w http.ResponseWriter, r *http.Request) {
}

if err == nil && attempts.LockedUntil.Valid && attempts.LockedUntil.Time.After(time.Now()) {
auditLogger.LogEvent(r.Context(), "user.auth.failure", req.Username, auditLogger.ClientIP(r), map[string]interface{}{
"reason": "account_locked",
})
WriteJSONError(w, "Invalid credentials", http.StatusUnauthorized)
return
}
Expand All @@ -52,6 +59,9 @@ func (s *Server) loginHandler(w http.ResponseWriter, r *http.Request) {
user, err := s.dbQueries.GetUserByUsername(r.Context(), req.Username)
if err != nil {
s.recordFailedAttempt(r, req.Username)
auditLogger.LogEvent(r.Context(), "user.auth.failure", req.Username, auditLogger.ClientIP(r), map[string]interface{}{
"reason": "unknown_user",
})
WriteJSONError(w, "Invalid credentials", http.StatusUnauthorized)
return
}
Expand All @@ -60,6 +70,9 @@ func (s *Server) loginHandler(w http.ResponseWriter, r *http.Request) {
valid, err := auth.VerifyPassword(req.Password, user.PasswordHash)
if err != nil || !valid {
s.recordFailedAttempt(r, req.Username)
auditLogger.LogEvent(r.Context(), "user.auth.failure", req.Username, auditLogger.ClientIP(r), map[string]interface{}{
"reason": "invalid_password",
})
WriteJSONError(w, "Invalid credentials", http.StatusUnauthorized)
return
}
Expand Down
9 changes: 9 additions & 0 deletions ground-control/internal/server/middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"strings"

"github.com/container-registry/harbor-satellite/ground-control/internal/auth"
auditLogger "github.com/container-registry/harbor-satellite/ground-control/internal/logger"
)

type contextKey string
Expand All @@ -25,6 +26,7 @@ func (s *Server) AuthMiddleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var user AuthUser
var authenticated bool
var actor string

// Try Bearer token first
if token := extractBearerToken(r); token != "" {
Expand All @@ -35,6 +37,7 @@ func (s *Server) AuthMiddleware(next http.Handler) http.Handler {
Username: session.Username,
Role: session.Role,
}
actor = session.Username
authenticated = true
}
}
Expand All @@ -51,13 +54,19 @@ func (s *Server) AuthMiddleware(next http.Handler) http.Handler {
Username: dbUser.Username,
Role: dbUser.Role,
}
actor = dbUser.Username
authenticated = true
}
}
}
}

if !authenticated {
auditLogger.LogEvent(r.Context(), "user.auth.failure", actor, auditLogger.ClientIP(r), map[string]interface{}{
"reason": "invalid_credentials",
"path": r.URL.Path,
"method": r.Method,
})
WriteJSONError(w, "Unauthorized", http.StatusUnauthorized)
return
}
Expand Down
10 changes: 10 additions & 0 deletions ground-control/internal/server/satellite_handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/container-registry/harbor-satellite/ground-control/internal/database"
auditLogger "github.com/container-registry/harbor-satellite/ground-control/internal/logger"
"github.com/container-registry/harbor-satellite/ground-control/internal/spiffe"
"github.com/container-registry/harbor-satellite/ground-control/internal/utils"
"github.com/container-registry/harbor-satellite/ground-control/reg/harbor"
Expand Down Expand Up @@ -241,6 +242,10 @@ func (s *Server) registerSatelliteHandler(w http.ResponseWriter, r *http.Request
}
committed = true

auditLogger.LogEvent(r.Context(), "satellite.registration", req.Name, auditLogger.ClientIP(r), map[string]interface{}{
"config_name": req.ConfigName,
})

resp := RegisterSatelliteResponse{
Token: tk,
}
Expand All @@ -259,6 +264,9 @@ func (s *Server) ztrHandler(w http.ResponseWriter, r *http.Request) {
if err != nil {
masked := maskToken(token)
log.Printf("Invalid Satellite Token %s: %v", masked, err)
auditLogger.LogEvent(r.Context(), "satellite.auth.failure", "", auditLogger.ClientIP(r), map[string]interface{}{
"reason": "invalid_token",
})
HandleAppError(w, &AppError{
Message: "Error: Invalid Token",
Code: http.StatusBadRequest,
Expand Down Expand Up @@ -920,6 +928,8 @@ func (s *Server) DeleteSatelliteByName(w http.ResponseWriter, r *http.Request) {
}
committed = true

auditLogger.LogEvent(r.Context(), "satellite.deregistration", satellite, auditLogger.ClientIP(r), nil)

WriteJSONResponse(w, http.StatusOK, map[string]string{})
}

Expand Down
9 changes: 9 additions & 0 deletions ground-control/internal/server/user_handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/container-registry/harbor-satellite/ground-control/internal/auth"
"github.com/container-registry/harbor-satellite/ground-control/internal/database"
auditLogger "github.com/container-registry/harbor-satellite/ground-control/internal/logger"
)

const (
Expand Down Expand Up @@ -83,6 +84,8 @@ func (s *Server) createUserHandler(w http.ResponseWriter, r *http.Request) {
return
}

auditLogger.LogEvent(r.Context(), "user.create", req.Username, auditLogger.ClientIP(r), nil)

WriteJSONResponse(w, http.StatusCreated, userResponse{
ID: user.ID,
Username: user.Username,
Expand Down Expand Up @@ -185,6 +188,8 @@ func (s *Server) deleteUserHandler(w http.ResponseWriter, r *http.Request) {
}

w.WriteHeader(http.StatusNoContent)

auditLogger.LogEvent(r.Context(), "user.delete", username, auditLogger.ClientIP(r), nil)
}

// changeOwnPasswordHandler allows any authenticated user to change their password
Expand Down Expand Up @@ -240,6 +245,8 @@ func (s *Server) changeOwnPasswordHandler(w http.ResponseWriter, r *http.Request
}

w.WriteHeader(http.StatusNoContent)

auditLogger.LogEvent(r.Context(), "user.password.change", currentUser.Username, auditLogger.ClientIP(r), nil)
}

// changeUserPasswordHandler allows system_admin to change any user's password
Expand Down Expand Up @@ -290,4 +297,6 @@ func (s *Server) changeUserPasswordHandler(w http.ResponseWriter, r *http.Reques
}

w.WriteHeader(http.StatusNoContent)

auditLogger.LogEvent(r.Context(), "user.password.change", username, auditLogger.ClientIP(r), nil)
}
86 changes: 86 additions & 0 deletions internal/logger/audit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package logger

import (
"io"
"os"
"sync"
"time"

"github.com/google/uuid"
"github.com/rs/zerolog"
)

// AuditEvent represents a security-relevant audit log entry.
type AuditEvent struct {
EventID string `json:"event_id"`
Timestamp time.Time `json:"timestamp"`
EventType string `json:"event_type"`
Actor string `json:"actor,omitempty"`
SourceIP string `json:"source_ip,omitempty"`
Details map[string]interface{} `json:"details,omitempty"`
}

var (
auditLogger *zerolog.Logger
auditLoggerOnce sync.Once
)
Comment on lines +23 to +26
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Remove package-level audit logger globals (lint violation).

Strict golangci-lint disallows package-level globals. Please refactor to an injected/instance-based logger.

As per coding guidelines: no global variables (gochecknoglobals).

🤖 Prompt for AI Agents 
In `@internal/logger/audit.go` around lines 23 - 26, Remove the package-level
globals auditLogger and auditLoggerOnce and refactor to an instance-based
logger: create an AuditLogger struct that holds a *zerolog.Logger, add a
constructor NewAuditLogger(logger *zerolog.Logger) (or accept a factory) and
convert any package-level functions that used auditLogger to methods on
*AuditLogger so callers inject or create the instance. Replace singleton
lazy-init logic (auditLoggerOnce) by requiring the caller to provide a
configured *zerolog.Logger (or by making NewAuditLogger initialize it once per
instance), update all call sites to use the AuditLogger receiver instead of the
globals, and ensure thread-safety is preserved by avoiding shared mutable
package state.


// InitAuditLogger configures the global audit logger.
// If writer is nil, audit logging is disabled.
func InitAuditLogger(writer io.Writer) {
if writer == nil {
return
}

auditLoggerOnce.Do(func() {
l := zerolog.New(writer).With().Timestamp().Logger()
auditLogger = &l
})
}

// LogAuditEvent writes a structured audit event if the audit logger is configured.
func LogAuditEvent(eventType, actor, sourceIP string, details map[string]interface{}) {
if auditLogger == nil {
return
}

event := AuditEvent{
EventID: uuid.NewString(),
Timestamp: time.Now().UTC(),
EventType: eventType,
Actor: actor,
SourceIP: sourceIP,
Details: details,
}

e := auditLogger.Info().
Str("event_id", event.EventID).
Time("timestamp", event.Timestamp).
Str("event_type", event.EventType)

if event.Actor != "" {
e = e.Str("actor", event.Actor)
}
if event.SourceIP != "" {
e = e.Str("source_ip", event.SourceIP)
}
if len(event.Details) > 0 {
e = e.Fields(event.Details)
}

e.Msg("")
}

// NewFileAuditWriter returns a basic file writer for audit logs.
// Rotation is expected to be handled by external logrotate where used.
func NewFileAuditWriter(path string) (io.Writer, error) {
if path == "" {
return nil, nil
}
f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600)
if err != nil {
return nil, err
}
return f, nil
}

1 change: 1 addition & 0 deletions pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type AppConfig struct {
TLS TLSConfig `json:"tls,omitempty"`
SPIFFE SPIFFEConfig `json:"spiffe,omitempty"`
EncryptConfig bool `json:"encrypt_config,omitempty"`
AuditLogPath string `json:"audit_log_path,omitempty"`
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for AuditLogPath references across the codebase
echo "=== Searching for AuditLogPath references ==="
rg -n "AuditLogPath|audit_log_path" -S

echo -e "\n=== Checking config.example.json ==="
fd -a 'config.example.json' -x cat {}

echo -e "\n=== Checking pkg/config/validate.go ==="
fd -a 'validate.go' pkg -x cat {}

Repository: container-registry/harbor-satellite

Length of output: 5460

Add AuditLogPath to validation and config example.

The AuditLogPath field added to AppConfig lacks validation in pkg/config/validate.go and is not documented in config.example.json. Add validation logic (path existence, permissions if needed) and include it in the example config so users can discover and configure this setting.

🤖 Prompt for AI Agents 
In `@pkg/config/config.go` at line 34, Add validation for the new
AppConfig.AuditLogPath by updating pkg/config/validate.go: inside the config
validation function that validates AppConfig (e.g., Validate or
ValidateAppConfig), check that AuditLogPath is non-empty when provided, that the
path exists and is a directory or file as expected, and that the process has
required permissions (read/write as appropriate); return a clear error if checks
fail. Also update the example configuration (config.example.json) to include the
AuditLogPath key with a descriptive sample value and brief comment so users can
discover and configure it.

}

type StateConfig struct {
Expand Down