Basekick-Labs / Basekick-Labs/arc
audit: middleware stores zero-copy Fiber strings in events that a background writer serializes later
- Dominant language
- Go
- Stars
- 677
- Forks
- 53
- Avg merge
- 9h 14m
- Merged PRs (30d)
- 164
Description
Found while reviewing #830.
## Problem
`internal/audit/middleware.go` builds an `AuditEvent` from `c.Path()`, `c.IP()`, `c.Get("User-Agent")`, `c.Get("x-arc-database")`, `c.Params(...)` and `c.Query("db")`, then hands the event to `Logger.LogEvent`, which only enqueues the pointer on a channel. The batch writer (`writerLoop`) serializes the fields up to a second later.
Arc runs Fiber with `Immutable=false` (see the note in `internal/api/msgpack.go`), so every one of those accessors returns a string that aliases a pooled buffer: `c.Query` is `app.getString(QueryArgs().Peek(key))`, `c.Path()` is `app.getString(c.pathBuffer)` on the pooled `Ctx`, and `c.Get` reads the request header buffer. Once the handler returns, the connection's `RequestCtx` and the `Ctx` go back to their pools and the next request overwrites those bytes. An audit row can therefore record a later request's path, database or user agent, or garbage. #830 adds `path` and `reason` from `c.Query` to the same event through `audit.DetailLocalsKey`, which the review asks to clone in the handler.
This is the same class as the retained `c.Get`/`c.Query` aliasing found in the edge-sync receive path (#776/#783 reviews).
## Fix shape
Copy every request-derived string when the event is built: `utils.CopyString` from `github.com/gofiber/fiber/v2/utils` on `path`, `c.IP()`, the user agent (after truncation), database, measurement, and any map supplied through the locals key (copy keys and values into a fresh map). `tokenInfo.Name` comes from the auth cache and needs no copy. Add a test that sends two different requests over one connection with `app.Test` disabled in favor of a real listener, or unit-test the copy by mutating the source buffer after the event is built and asserting the event is unchanged.
Small, self-contained, and a good first issue once the recipe above is followed exactly.
Contributor guide
Research direction
Start in internal/audit/middleware.go and trace how request-derived values enter AuditEvent before Logger.LogEvent queues it; check the Immutable=false context in internal/api/msgpack.go and the locals supplied through audit.DetailLocalsKey. Copy the listed strings and map data when building the event, then verify with either a real-listener two-request test or a source-buffer mutation test that serialized events remain unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, observability
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 82/100