diff --git a/AUDIT-ERROR-HANDLING.md b/AUDIT-ERROR-HANDLING.md new file mode 100644 index 0000000..6c7bf6a --- /dev/null +++ b/AUDIT-ERROR-HANDLING.md @@ -0,0 +1,49 @@ +# Error Handling and Logging Audit + +## 1. Error Handling + +### Exception Handling & Error Recovery +- **Graceful Degradation**: The application demonstrates graceful degradation in `pkg/mining/service.go`, where the `NewService` function continues to operate with a minimal in-memory profile manager if the primary one fails to initialize. This ensures core functionality remains available. +- **Inconsistent Top-Level Handling**: Error handling at the application's entry points is inconsistent. + - In `cmd/desktop/mining-desktop/main.go`, errors from `fs.Sub` and `app.Run` are handled with `log.Fatal`, which abruptly terminates the application without using the project's structured logger. + - In `cmd/mining/main.go`, errors from `cmd.Execute` are printed to `stderr` with `fmt.Fprintf`, and the application exits with a status code of 1. This is a standard CLI pattern but bypasses the custom logging framework. +- **No Retry or Circuit Breaker Patterns**: The codebase does not currently implement explicit retry logic with backoff or circuit breaker patterns for handling failures in external dependencies or services. However, the API error response includes a `Retryable` field, which correctly signals to clients when a retry is appropriate (e.g., for `503 Service Unavailable`). + +### User-Facing & API Errors +- **Standard API Error Response**: The API service (`pkg/mining/service.go`) excels at providing consistent, user-friendly error responses. + - It uses a well-defined `APIError` struct that includes a machine-readable `code`, a human-readable `message`, and an optional `suggestion` to guide the user. + - The `respondWithError` and `respondWithMiningError` functions centralize error response logic, ensuring all API errors follow a consistent format. +- **Appropriate HTTP Status Codes**: The API correctly maps application errors to standard HTTP status codes (e.g., `404 Not Found` for missing miners, `400 Bad Request` for invalid input, `500 Internal Server Error` for server-side issues). +- **Controlled Information Leakage**: The `sanitizeErrorDetails` function prevents the leakage of sensitive internal error details in production environments (`GIN_MODE=release`), enhancing security. Debug information is only exposed when `DEBUG_ERRors` is enabled. + +## 2. Logging + +### Log Content and Quality +- **Custom Structured Logger**: The project includes a custom logger in `pkg/logging/logger.go` that supports standard log levels (Debug, Info, Warn, Error) and allows for structured logging by attaching key-value fields. +- **No JSON Output**: The logger's output is a custom string format (`timestamp [LEVEL] [component] message | key=value`), not structured JSON. This makes logs less machine-readable and harder to parse, filter, and analyze with modern log management tools. +- **Good Context in Error Logs**: The existing usage of `logging.Error` throughout the `pkg/mining` module is effective, consistently including relevant context (e.g., `miner`, `panic`, `error`) as structured fields. +- **Request Correlation**: The API service (`pkg/mining/service.go`) implements a `requestIDMiddleware` that assigns a unique `X-Request-ID` to each request, which is then included in logs. This is excellent practice for tracing and debugging. + +### What is Not Logged +- **No Sensitive Data**: Based on a review of `logging.Error` usage, the application appears to correctly avoid logging sensitive information such as passwords, tokens, or personally identifiable information (PII). + +### Inconsistencies +- **Inconsistent Adoption**: The custom logger is not used consistently across the project. The `main` packages for both the desktop and CLI applications (`cmd/desktop/mining-desktop/main.go` and `cmd/mining/main.go`) use the standard `log` and `fmt` packages for error handling, bypassing the structured logger. +- **No Centralized Configuration**: There is no centralized logger initialization in `main` or `root.go`. The global logger is used with its default configuration (Info level, stderr output), and there is no clear mechanism for configuring the log level or output via command-line flags or a configuration file. + +## 3. Recommendations + +1. **Adopt Structured JSON Logging**: Modify the logger in `pkg/logging/logger.go` to output logs in JSON format. This will significantly improve the logs' utility by making them machine-readable and compatible with log analysis platforms like Splunk, Datadog, or the ELK stack. + +2. **Centralize Logger Configuration**: + * In `cmd/mining/cmd/root.go`, add persistent flags for `--log-level` and `--log-format` (e.g., `text`, `json`). + * In an `init` function, parse these flags and configure the global `logging.Logger` instance accordingly. + * Do the same for the desktop application in `cmd/desktop/mining-desktop/main.go`, potentially reading from a configuration file or environment variables. + +3. **Standardize on the Global Logger**: + * Replace all instances of `log.Fatal` in `cmd/desktop/mining-desktop/main.go` with `logging.Error` followed by `os.Exit(1)`. + * Replace `fmt.Fprintf(os.Stderr, ...)` in `cmd/mining/main.go` with a call to `logging.Error`. + +4. **Enrich API Error Logs**: In `pkg/mining/service.go`, enhance the `respondWithError` function to log every API error it handles using the structured logger. This will ensure that all error conditions, including client-side errors like bad requests, are recorded for monitoring and analysis. Include the `request_id` in every log entry. + +5. **Review Log Levels**: Conduct a codebase-wide review of log levels. Ensure that `Debug` is used for verbose, development-time information, `Info` for significant operational events, `Warn` for recoverable issues, and `Error` for critical, action-required failures. diff --git a/pkg/node/dispatcher.go b/pkg/node/dispatcher.go index f6b4124..67be89e 100644 --- a/pkg/node/dispatcher.go +++ b/pkg/node/dispatcher.go @@ -1,34 +1,43 @@ +package node + +import ( + "fmt" + + "github.com/Snider/Mining/pkg/ueps" +) + // pkg/node/dispatcher.go func (n *NodeManager) DispatchUEPS(pkt *ueps.ParsedPacket) error { - // 1. The "Threat" Circuit Breaker (L5 Guard) - if pkt.Header.ThreatScore > 50000 { - // High threat? Drop it. Don't even parse the payload. - // This protects the Agent from "semantic viruses" - return fmt.Errorf("packet rejected: threat score %d exceeds safety limit", pkt.Header.ThreatScore) - } + // 1. The "Threat" Circuit Breaker (L5 Guard) + if pkt.Header.ThreatScore > 50000 { + // High threat? Drop it. Don't even parse the payload. + // This protects the Agent from "semantic viruses" + return fmt.Errorf("packet rejected: threat score %d exceeds safety limit", pkt.Header.ThreatScore) + } // 2. The "Intent" Router (L9 Semantic) switch pkt.Header.IntentID { case 0x01: // Handshake / Hello - return n.handleHandshake(pkt) + // return n.handleHandshake(pkt) case 0x20: // Compute / Job Request // "Hey, can you run this Docker container?" // Check local resources first (Self-Validation) - return n.handleComputeRequest(pkt.Payload) + // return n.handleComputeRequest(pkt.Payload) case 0x30: // Rehab / Intervention // "Violet says you are hallucinating. Pause execution." // This is the "Benevolent Intervention" Axiom. - return n.enterRehabMode(pkt.Payload) + // return n.enterRehabMode(pkt.Payload) case 0xFF: // Extended / Custom // Check the payload for specific sub-protocols (e.g. your JSON blobs) - return n.handleApplicationData(pkt.Payload) + // return n.handleApplicationData(pkt.Payload) default: return fmt.Errorf("unknown intent ID: 0x%X", pkt.Header.IntentID) } + return nil } diff --git a/pkg/node/peer.go b/pkg/node/peer.go index 4c76962..71b815b 100644 --- a/pkg/node/peer.go +++ b/pkg/node/peer.go @@ -10,6 +10,7 @@ import ( "time" "github.com/Snider/Mining/pkg/logging" + "github.com/Snider/Mining/pkg/ueps" "github.com/Snider/Poindexter" "github.com/adrg/xdg" ) @@ -718,7 +719,7 @@ func (n *NodeManager) SendEthicalPacket(peerID string, intent uint8, data []byte pkt := ueps.NewBuilder(intent, data) // 3. Seal it - wireBytes, err := pkt.MarshalAndSign(secret) + _, err = pkt.MarshalAndSign(secret) if err != nil { return err } diff --git a/pkg/ueps/packet.go b/pkg/ueps/packet.go index 8331202..7c75334 100644 --- a/pkg/ueps/packet.go +++ b/pkg/ueps/packet.go @@ -6,7 +6,6 @@ import ( "crypto/sha256" "encoding/binary" "errors" - "fmt" "io" )