Skip to content

Commit 42452e3

Browse files
tire-firetire-fire
authored
Generate request IDs when none is set (#182)
* Bump golangci-lint to v2.15 and gosec to v2.30.0 for Go 1.27 * Generate request IDs when none is set * Shorten request IDs and store them under a typed key
1 parent e595b0f commit 42452e3

7 files changed

Lines changed: 62 additions & 21 deletions

File tree

‎www/api/api.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func WriteJSON(w http.ResponseWriter, status int, data any) {
4040

4141
// WriteInternalError logs err and sends only msg to the client.
4242
func WriteInternalError(w http.ResponseWriter, r *http.Request, msg string, err error) {
43-
slog.Error(msg, "request_id", r.Context().Value("request_id"), "error", err.Error())
43+
slog.Error(msg, "request_id", RequestID(r.Context()), "error", err.Error())
4444
WriteJSON(w, http.StatusInternalServerError, map[string]any{"error": msg})
4545
}
4646

‎www/api/identity.go‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,22 @@ type Identity struct {
3434

3535
type contextKey int
3636

37-
const identityContextKey contextKey = iota
37+
const (
38+
identityContextKey contextKey = iota
39+
requestIDContextKey
40+
)
41+
42+
// WithRequestID stores the request ID on the context. Called only by the
43+
// logging middleware.
44+
func WithRequestID(ctx context.Context, id string) context.Context {
45+
return context.WithValue(ctx, requestIDContextKey, id)
46+
}
47+
48+
// RequestID returns the request's ID, or "" if none was set.
49+
func RequestID(ctx context.Context) string {
50+
id, _ := ctx.Value(requestIDContextKey).(string)
51+
return id
52+
}
3853

3954
// WithIdentity stores the identity on the request context. Called only by the
4055
// authentication middleware.

‎www/api/pcrs.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ func (a *API) CreatePcr(w http.ResponseWriter, r *http.Request) {
172172
err := decoder.Decode(&form)
173173
if err != nil {
174174
WriteJSON(w, http.StatusBadRequest, map[string]any{"error": "Invalid request body"})
175-
slog.Error("Failed to decode PCR json", "request_id", r.Context().Value("request_id"), "error", err.Error())
175+
slog.Error("Failed to decode PCR json", "request_id", RequestID(r.Context()), "error", err.Error())
176176
return
177177
}
178178

@@ -217,7 +217,7 @@ func (a *API) ResetPcr(w http.ResponseWriter, r *http.Request) {
217217
err := decoder.Decode(&form)
218218
if err != nil {
219219
WriteJSON(w, http.StatusBadRequest, map[string]any{"error": "Invalid request body"})
220-
slog.Error("Failed to decode PCR json", "request_id", r.Context().Value("request_id"), "error", err.Error())
220+
slog.Error("Failed to decode PCR json", "request_id", RequestID(r.Context()), "error", err.Error())
221221
return
222222
}
223223
teamID, ok := a.pcrTeamID(w, r, form.TeamID, "PCR reset not allowed")

‎www/api/red.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func (a *API) EditBox(w http.ResponseWriter, r *http.Request) {
7575
var id uint
7676
if temp, err := strconv.ParseUint(r.FormValue("box-id"), 10, 64); err != nil {
7777
WriteJSON(w, http.StatusBadRequest, map[string]any{"error": "Failed to convert box id"})
78-
slog.Error("", "request_id", r.Context().Value("request_id"), "error", err.Error())
78+
slog.Error("", "request_id", RequestID(r.Context()), "error", err.Error())
7979
return
8080
} else {
8181
id = uint(temp)
@@ -114,7 +114,7 @@ func (a *API) CreateVector(w http.ResponseWriter, r *http.Request) {
114114
var vuln uint
115115
if v, err := strconv.Atoi(vulnID); err != nil {
116116
WriteJSON(w, http.StatusBadRequest, map[string]any{"error": "Failed to convert vuln id"})
117-
slog.Error("", "request_id", r.Context().Value("request_id"), "error", err.Error())
117+
slog.Error("", "request_id", RequestID(r.Context()), "error", err.Error())
118118
return
119119
} else if v < 0 {
120120
WriteJSON(w, http.StatusBadRequest, map[string]any{"error": "Vuln ID must be non-negative"})
@@ -126,7 +126,7 @@ func (a *API) CreateVector(w http.ResponseWriter, r *http.Request) {
126126
var box uint
127127
if v, err := strconv.Atoi(boxID); err != nil {
128128
WriteJSON(w, http.StatusBadRequest, map[string]any{"error": "Failed to convert box id"})
129-
slog.Error("", "request_id", r.Context().Value("request_id"), "error", err.Error())
129+
slog.Error("", "request_id", RequestID(r.Context()), "error", err.Error())
130130
return
131131
} else if v < 0 {
132132
WriteJSON(w, http.StatusBadRequest, map[string]any{"error": "Box ID must be non-negative"})
@@ -138,7 +138,7 @@ func (a *API) CreateVector(w http.ResponseWriter, r *http.Request) {
138138
port, err := strconv.Atoi(portStr)
139139
if err != nil {
140140
WriteJSON(w, http.StatusBadRequest, map[string]any{"error": "Failed to convert port"})
141-
slog.Error("", "request_id", r.Context().Value("request_id"), "error", err.Error())
141+
slog.Error("", "request_id", RequestID(r.Context()), "error", err.Error())
142142
return
143143
}
144144
if port < 0 || port > 65535 {
@@ -170,7 +170,7 @@ func (a *API) CreateAttack(w http.ResponseWriter, r *http.Request) {
170170
r.Body = http.MaxBytesReader(w, r.Body, 10<<20)
171171
if err := r.ParseMultipartForm(10 << 20); err != nil {
172172
WriteJSON(w, http.StatusBadRequest, map[string]any{"error": "Failed to parse multipart form"})
173-
slog.Error("", "request_id", r.Context().Value("request_id"), "error", err.Error())
173+
slog.Error("", "request_id", RequestID(r.Context()), "error", err.Error())
174174
return
175175
}
176176

@@ -194,7 +194,7 @@ func (a *API) CreateAttack(w http.ResponseWriter, r *http.Request) {
194194
var vector uint
195195
if v, err := strconv.Atoi(vectorID); err != nil {
196196
WriteJSON(w, http.StatusBadRequest, map[string]any{"error": "Failed to convert vector id"})
197-
slog.Error("", "request_id", r.Context().Value("request_id"), "error", err.Error())
197+
slog.Error("", "request_id", RequestID(r.Context()), "error", err.Error())
198198
return
199199
} else if v < 0 {
200200
WriteJSON(w, http.StatusBadRequest, map[string]any{"error": "Vector ID must be non-negative"})
@@ -206,7 +206,7 @@ func (a *API) CreateAttack(w http.ResponseWriter, r *http.Request) {
206206
var team uint
207207
if v, err := strconv.Atoi(teamID); err != nil {
208208
WriteJSON(w, http.StatusBadRequest, map[string]any{"error": "Failed to convert team id"})
209-
slog.Error("", "request_id", r.Context().Value("request_id"), "error", err.Error())
209+
slog.Error("", "request_id", RequestID(r.Context()), "error", err.Error())
210210
return
211211
} else if v < 0 {
212212
WriteJSON(w, http.StatusBadRequest, map[string]any{"error": "Team ID must be non-negative"})
@@ -218,7 +218,7 @@ func (a *API) CreateAttack(w http.ResponseWriter, r *http.Request) {
218218
access, err := strconv.Atoi(accessLevelStr)
219219
if err != nil {
220220
WriteJSON(w, http.StatusBadRequest, map[string]any{"error": "Failed to convert access level"})
221-
slog.Error("", "request_id", r.Context().Value("request_id"), "error", err.Error())
221+
slog.Error("", "request_id", RequestID(r.Context()), "error", err.Error())
222222
return
223223
}
224224

‎www/api/timelapse.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func (a *API) ExportTimelapse(w http.ResponseWriter, r *http.Request) {
7676

7777
w.Header().Set("Content-Type", "image/gif")
7878
if err := tl.Encode(w); err != nil {
79-
slog.Error("failed to write timelapse", "request_id", r.Context().Value("request_id"), "error", err)
79+
slog.Error("failed to write timelapse", "request_id", RequestID(r.Context()), "error", err)
8080
}
8181
}
8282

‎www/middleware/logging.go‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,25 @@
11
package middleware
22

33
import (
4-
"context"
4+
"crypto/rand"
55
"log/slog"
66
"net/http"
77
"time"
88

99
"github.com/dbaseqp/Quotient/www/api"
10-
"github.com/google/uuid"
1110
)
1211

1312
// Logging middleware logs quotient web requests. It is meant to be called after authentication is performed
1413
func Logging(next http.HandlerFunc) http.HandlerFunc {
1514
return func(w http.ResponseWriter, r *http.Request) {
1615
start := time.Now()
1716

18-
// Generate (or reuse) a unique request ID.
19-
requestID := r.Context().Value("request_id")
17+
// Generate (or reuse) a request ID.
18+
requestID := api.RequestID(r.Context())
2019
if requestID == "" {
21-
requestID = uuid.New().String()
22-
// TODO: use custom type to fix staticcheck instead
23-
// nolint:staticcheck
24-
r = r.WithContext(context.WithValue(r.Context(), "request_id", requestID))
20+
// Short enough to keep log lines readable; only needs to tell nearby requests apart.
21+
requestID = rand.Text()[:8]
22+
r = r.WithContext(api.WithRequestID(r.Context(), requestID))
2523
}
2624

2725
// Process the request.

‎www/middleware/logging_test.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package middleware
2+
3+
import (
4+
"context"
5+
"net/http"
6+
"net/http/httptest"
7+
"testing"
8+
9+
"github.com/dbaseqp/Quotient/www/api"
10+
11+
"github.com/stretchr/testify/assert"
12+
)
13+
14+
func TestLoggingSetsRequestID(t *testing.T) {
15+
var seen []string
16+
handler := Logging(func(w http.ResponseWriter, r *http.Request) {
17+
seen = append(seen, api.RequestID(r.Context()))
18+
})
19+
20+
handler(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/", nil))
21+
handler(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/", nil))
22+
assert.Len(t, seen[0], 8, "the handler sees a request ID")
23+
assert.NotEqual(t, seen[0], seen[1], "each request gets its own ID")
24+
25+
ctx := api.WithRequestID(context.Background(), "existing")
26+
handler(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/", nil).WithContext(ctx))
27+
assert.Equal(t, "existing", seen[2], "an existing request ID is kept")
28+
}

0 commit comments

Comments
 (0)